[Chronik] Fix incorrectly compressed P2PK scripts
ClosedPublic
Actions

Authored by tobias_ruck on Sat, Oct 19, 20:37.

Details

Reviewers

PiRK

Group Reviewers

Restricted Project

Commits

rABC9032f454dcf8: [Chronik] Fix incorrectly compressed P2PK scripts

Summary

This iterates through all scripts and finds incorrectly compressed scripts.

Only keys in the DB that are 33 bytes long are affected, and only keys that don't start with 0x02 or 0x03.

It also turns out that there's no keys with 0x00 and 0x01 that are affected.

However, we scan the entire database anyway, as between now and the upgrade, someone could create txs with 33 byte long pubkeys that e.g. start with 0 or 1. Note that these txs are standard (c.f. MatchPayToPubkey), so this is definitely non-optional.

On my high-end instance, the update takes 5m:17s, so on bad hardware it shouldn't take more than 15 minutes.

At the time of writing, the only affected txs are these:

00a17fbae6ee18cf7f645f45f30ed7abd9c39a551e3043fab15433f4e210590e
237783998a6799264983150187a73ab6d116f2ba78d3e1f88529e95229f59d67
3382a0d3fba1f62e52fe37cb0b7733d35ca11eaff200b9b2993d92301bf816ae
347939105c1c90282698ef83d59d525aa5022002318949cb6c9dde271535432f
52e307670ad54db6490f8ac11fe456a71dd0ef8f9fb1e52b1248f25eb69bb504
5d8403451c6313c0e9f41746af8a86a8929c331f1e01331726fad9166a07ba7a
6450378a5ab233a0fdb76e24903a17f9771c5f38d175fc611e6e98bdfbd7764f
64bd61eedfef518e995c421bbd9dab99553a7787f74f1afddf1c33d1b947ec41
65650c0b17d063ede2bfa28d1530f31b0bb1e0413849b1e31bde8f5adb6af891
657aecafe66d729d2e2f6f325fcc4acb8501d8f02512d1f5042a36dd1bbd21d1
71bbaef28e09d8d6fadd41f053db7768dbb5fa4570f06b961dfc29db3dc00b1d
77445dd56cea3173e957db23060380fe99a01bab570d93226f831252f85cea41
846692c201067961c84c397e635a14a8b776a0b1c9dc89a8e24f472700e56e2a
b57168c6b9bafe1a7a14ffdd3e4069d88e77f5ef41b931d25205b96c309526cf
ef4bcadfbcb8c3a7c734ce957b5078d760229b93be41091918c2b1e18bd09807

Depends on D16936.

Test Plan

Make a backup of the Chronik DB
Search for "UPGRADE13", apply the changes
Build using ninja
Run node (using -debug=chronik), it should upgrade automatically to 13
It should print "Fixing P2PK scripts. This may take a while on slow hardware"
It should print "Tx {txid} has incorrectly compressed P2PK script" for all the txs in the summary
Upgrade should be successful

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

tobias_ruck created this revision.Sat, Oct 19, 20:37

Herald added a reviewer: Restricted Project. · View Herald TranscriptSat, Oct 19, 20:37

Harbormaster completed remote builds in B30675: Diff 50206.Sat, Oct 19, 20:43

Build Bitcoin ABC Diffs / Diff Testing (build-chronik) failed.

Tail of the build log:

   Doc-tests chronik-bridge

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-db

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-http

running 1 test
test chronik/chronik-http/src/protobuf.rs - protobuf::Protobuf (line 29) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.33s

   Doc-tests chronik-indexer

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin-common

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin-impl

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-proto

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-util

running 2 tests
test chronik/chronik-util/src/log.rs - log::log (line 65) ... ignored
test chronik/chronik-util/src/log.rs - log::log_chronik (line 87) ... ignored

test result: ok. 0 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out; finished in 0.00s

ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik failed with exit code 1

Build Bitcoin ABC Diffs / Diff Testing (build-chronik-plugins) failed.

Tail of the build log:

   Doc-tests chronik-bridge

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-db

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-http

running 1 test
test chronik/chronik-http/src/protobuf.rs - protobuf::Protobuf (line 29) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.33s

   Doc-tests chronik-indexer

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin-common

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-plugin-impl

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-proto

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests chronik-util

running 2 tests
test chronik/chronik-util/src/log.rs - log::log (line 65) ... ignored
test chronik/chronik-util/src/log.rs - log::log_chronik (line 87) ... ignored

test result: ok. 0 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out; finished in 0.00s

ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik-plugins failed with exit code 1

tobias_ruck edited the summary of this revision. (Show Details)Sat, Oct 19, 22:15

tobias_ruck added a child revision: D16941: [Chronik] Clear OP_RETURN scripts from script index, enable DB upgrade to version 13.Sat, Oct 19, 22:41

tobias_ruck edited the summary of this revision. (Show Details)Sat, Oct 19, 22:56

fix clippy

Harbormaster completed remote builds in B30688: Diff 50220.Sun, Oct 20, 00:00

tobias_ruck published this revision for review.Sun, Oct 20, 00:14

PiRK accepted this revision.Mon, Oct 21, 08:51

PiRK added a subscriber: PiRK.

PiRK added inline comments.

chronik/chronik-db/src/io/upgrade.rs
324 ↗	(On Diff #50220)	This log line is probably just going to be confusing the user who does not have the context to know what ambiguous refers to in this context. I would only print this info if `num_scripts_both > 0` in the below `if` (if the Err does not already trigger a log line).

This revision is now accepted and ready to land.Mon, Oct 21, 08:51

tobias_ruck added inline comments.Mon, Oct 21, 08:53

chronik/chronik-db/src/io/upgrade.rs
324 ↗	(On Diff #50220)	Well it's only logged with -debug=chronik. And if the error occurs it'll be logged there anyway So I can remove it if you think it's too weird

PiRK added inline comments.Mon, Oct 21, 12:05

chronik/chronik-db/src/io/upgrade.rs
324 ↗	(On Diff #50220)	If it is only a debugging log, it doesn't really matter. I expect most users who enable debug logs are capable of looking at the code to find out what it is about.