test: Validate UTXO snapshot with coin height > base height & amount > MAX_MONEY supply #29617

jrakibi · 2024-03-11T03:00:51Z

Ensure snapshot loading fails for coins exceeding base height

Objective: This test verifies that snapshot loading is correctly rejected for coins with a height greater than the base height.

Update:

Added test_invalid_snapshot_wrong_coin_code to feature_assumeutxo.py.
The test artificially sets a coin's height above 299 in a snapshot and checks for load failure.
Edit: Added a test case for outputs whose amounts surpass the MAX_MONEY supply limit.

This implementation addresses the request for enhancing assumeutxo testing as outlined in issue #28648

Edit: This is an explanation on how I arrive at content values: b"\x84\x58" and b"\xCA\xD2\x8F\x5A"

You can use this tool to decode the utxo snapshot https://github.com/jrakibi/utxo-live
Here’s an overview of how it’s done:
The serialization format for a UTXO in the snapshot is as follows:

Transaction ID (txid) - 32 bytes
Output Index (outnum)- 4 bytes
VARINT (code) - A varible-length integer encoding the height and whether the transaction is a coinbase. The format of this VARINT is (height << 1) | coinbase_flag.
VARINT (amount_v) - A variable-length integer that represents a compressed format of the output amount (in satoshis).

For the test cases mentioned:

b"\x84\x58" - This value corresponds to a VARINT representing the height and coinbase flag. Once we decode this code, we can extract the height and coinbase using height = code_decoded >> 1 and coinbase = code_decoded & 0x01. In our case, with code_decoded = 728, it results in height = 364 and coinbase = 0.
b"\xCA\xD2\x8F\x5A" - This byte sequence represents a compressed amount value. The decompression function takes this value and translates it into a full amount in satoshis. In our case, the decompression of this amount translates to a number larger than the maximum allowed value of coins (21 million BTC)

DrahtBot · 2024-03-11T03:00:54Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	fjahr, maflcko, achow101
Stale ACK	rkrux

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#29612 (rpc: Optimize serialization and enhance metadata of dumptxoutset output by fjahr)
#27432 (contrib: add tool to convert compact-serialized UTXO set to SQLite database by theStack)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot · 2024-03-11T04:05:48Z

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

_{Debug: https://github.com/bitcoin/bitcoin/runs/22494357167}

BrandonOdiwuor

You should remove the TODO:

bitcoin/test/functional/feature_assumeutxo.py

Lines 19 to 20 in a945f09

- TODO: Valid hash but invalid snapshot file (bad coin height or

bad other serialization)

test/functional/feature_assumeutxo.py

ismaelsadeeq

First pass Partial review.

Please squash the two commits https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
From the PR OP and description you mention base height, I am not familiar with the term "base height" did you mean chain tip?

ismaelsadeeq · 2024-03-11T15:50:14Z

test/functional/feature_assumeutxo.py

Suggested change

"Testing snapshot file with an incorrect coin code: height 364 and coinbase 0"

"Testing snapshot file with an incorrect coin height 364 and coinbase 0"

Thanks @ismaelsadeeq, I've squashed them.
Base height is related to AssumeUTXO snapshot, refers to the block height at which the UTXOset snapshot was taken.
Also coin code in this context encodes the block height and whether the tx is Coinbase or not

test/functional/feature_assumeutxo.py

maflcko · 2024-03-11T16:02:17Z

Missing test: prefix in pull title?

DrahtBot · 2024-03-13T00:01:40Z

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

_{Debug: https://github.com/bitcoin/bitcoin/runs/22587330142}

jrakibi · 2024-03-13T00:47:12Z

@ismaelsadeeq @BrandonOdiwuor I've updated the PR with your feedbacks. Also included my test case in test_invalid_snapshot_scenarios method to keep all invalid snapshot scenarios cases in one place

jrakibi · 2024-03-19T00:41:17Z

I've just added another test case to invalidate snapshots containing outputs whose amounts exceed the MAX_MONEY supply limit

rkrux

tACK e807838

I was able to successfully build and run all the functional tests.
Provided a refactoring suggestion in favour of clear separation of variables.

test/functional/feature_assumeutxo.py

fjahr

Concept ACK

I think it would be helpful for reviewers if you could add information in the commit message how you arrived at the content values b"\x84\x58" and b"\xCA\xD2\x8F\x5A".

test/functional/feature_assumeutxo.py

jrakibi · 2024-04-22T13:48:22Z

@fjahr I've updated the code based on your feedback. In the commit & PR description, you can find the steps followed to arrive at the content values, along with a tool to decode a UTXO snapshot

fjahr

Code review ACK 6428b363e68fee51a455ebd4f9bf7ed6d813caa8

Ignore the nit unless there is other feedback to address or you need to rebase.

fjahr · 2024-04-22T15:45:39Z

test/functional/feature_assumeutxo.py

nit: exceeds

Fixed and rebased.

… money_supply You can use this tool to decode the utxo snapshot https://github.com/jrakibi/utxo-live Here’s an overview of how it’s done: The serialization forma for a UTXO in the snapshot is as follows: 1. Transaction ID (txid) - 32 bytes 2. Output Index (outnum)- 4 bytes 3. VARINT (code) - A varible-length integer encoding the height and whether the transaction is a coinbase. The format of this VARINT is (height << 1) | coinbase_flag. 4. VARINT (amount_v) - A variable-length integer that represents a compressed format of the output amount (in satoshis). For the test cases mentioned: * b"\x84\x58" - This value corresponds to a VARINT representing the height and coinbase flag. Once we decode this code, we can extract the height and coinbase using height = code_decoded >> 1 and coinbase = code_decoded & 0x01. In our case, with code_decoded = 728, it results in height = 364 and coinbase = 0. * b"\xCA\xD2\x8F\x5A" - This byte sequence represents a compressed amount value. The decompression function takes this value and translates it into a full amount in satoshis. In our case, the decompression of this amount translates to a number larger than the maximum allowed value of coins (21 million BTC) test:Validate UTXO snapshot with coin_height > base_height & amount > money_supply test:Validate UTXO snapshot with coin_height > base_height & amount > money_supply

fjahr · 2024-04-22T23:35:37Z

re-ACK ec1f1ab

maflcko · 2024-04-23T07:33:45Z

ACK ec1f1ab 👑

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK ec1f1abfefa281e62bb876aa1c4738d576ef9a4 👑
JmyXc4AA3GlfNbDIttEDoTvrUHdJtZaS+AtVKAl4yGV+WmcfcEFyZjxpaa5jgmWqix+7xHNhB/dd8/He/ZgGBg==

achow101 · 2024-05-02T20:38:55Z

ACK ec1f1ab

Summary: This is a partial backport of [[bitcoin/bitcoin#27596 | core#27596]], [[bitcoin/bitcoin#28652 | core#28652]], [[bitcoin/bitcoin#29345 | core#29345]], [[bitcoin/bitcoin#28589 | core#28589]], [[bitcoin/bitcoin#28590 | core#28590]], [[bitcoin/bitcoin#28625 | core#28625]], [[bitcoin/bitcoin#28639 | core#28639]], [[bitcoin/bitcoin#28647 | core#28647]], [[bitcoin/bitcoin#28562 | core#28562]], [[bitcoin/bitcoin#28666 | core#28666]], [[bitcoin/bitcoin#28669 | core#28669]], [[bitcoin/bitcoin#28685 | core#28685]], [[bitcoin/bitcoin#29215 | core#29215]], [[bitcoin/bitcoin#29726 | core#29726]], [[bitcoin/bitcoin#29354 | core#29354]], [[bitcoin/bitcoin#29394 | core#29394]], [[bitcoin/bitcoin#29478 | core#29478]], [[bitcoin/bitcoin#29370 | core#29370]], [[bitcoin/bitcoin#29617 | core#29617]], [[bitcoin/bitcoin#28685 | core#28685]], [[bitcoin/bitcoin#30053 | core#30053]], [[bitcoin/bitcoin#29973 | core#29973]], [[bitcoin/bitcoin#28838 | core#28838]], [[bitcoin/bitcoin#29354 | core#29354]], [[bitcoin/bitcoin#29478 | core#29478]], [[bitcoin/bitcoin#30678 | core#30678]], [[bitcoin/bitcoin#31556 | core#31556]] and [[bitcoin/bitcoin#30909 | core#30909]] Depends on D17945 ------ rpc: add loadtxoutset Co-authored-by: Sebastian Falbesoner <[email protected]> bitcoin/bitcoin@ce585a9 [[bitcoin/bitcoin#28652 | core#28652]] [[bitcoin/bitcoin#29345 | core#29345]] ------ rpc: add getchainstates Co-authored-by: Ryan Ofsky <[email protected]> bitcoin/bitcoin@0f64bac bitcoin/bitcoin@a9ef702 ---- test: add feature_assumeutxo functional test Initial commit: bitcoin/bitcoin@42cae39 [[bitcoin/bitcoin#28589 | core#28589]] (race fixes) [[bitcoin/bitcoin#28590 | core#28590]] (getchainstates return a list of chainstates) [[bitcoin/bitcoin#28625 | core#28625]] ( check that loading snapshot not matching AssumeUTXO parameters fails) bitcoin/bitcoin@fafde92 [[bitcoin/bitcoin#28647 | core#28647]] (Add assumeutxo test for wrong hash) [[bitcoin/bitcoin#28652 | core#28652]] (fail early if snapshot block hash doesn't match AssumeUTXO parameters) [[bitcoin/bitcoin#28562 | core#28562]] (`self.no_op`, `self.wait_until`) [[bitcoin/bitcoin#28666 | core#28666]] (assumeutxo file with unknown block hash) [[bitcoin/bitcoin#28669 | core#28669]] (check au file with changed outpoint index) [[bitcoin/bitcoin#28685 | core#28685]] (add tests for coin maleation) [[bitcoin/bitcoin#29215 | core#29215]] (spend coin from snapshot chainstate after loading) bitcoin/bitcoin@b7ba60f (add coverage for -reindex and assumeutxo) [[bitcoin/bitcoin#29354 | core#29354]] (Assumeutxo with more than just coinbase transactions) [[bitcoin/bitcoin#29394 | core#29394]] (Add test to ensure failure when mempool not empty) bitcoin/bitcoin@2bc1ecf (Remove unnecessary sync_blocks in assumeutxo tests) [[bitcoin/bitcoin#29478 | core#29478]] (Add test for loadtxoutset when headers are not synced) [[bitcoin/bitcoin#29370 | core#29370]] (RPC test for fake nTx and nChainTX values, stale block CheckBlockIndex crash test & assumeutxo snapshot block CheckBlockIndex crash test) [[bitcoin/bitcoin#29617 | core#29617]] (test for coin_height > base_height & amount > money_supply) bitcoin/bitcoin@f621392 (Check deserialized coins for out of range values) [[bitcoin/bitcoin#30053 | core#30053]] (coverage for "Couldn't open file..." error) [[bitcoin/bitcoin#29973 | core#29973]] (ensure failure when importing a snapshot twice) ------ test: add assumeutxo wallet test initial commit: [[bitcoin/bitcoin#28838 | core#28838]] bitcoin/bitcoin@fa5cd66 (Assumeutxo with more than just coinbase transactions) bitcoin/bitcoin@2bc1ecf (Remove unnecessary sync_blocks in assumeutxo tests) bitcoin/bitcoin@7e3dbe4 (functional test only) bitcoin/bitcoin@f20fe33 (Add basic balance coverage) bitcoin/bitcoin@bc43eca (test for balance after snapshot completion) bitcoin/bitcoin@595edee (import descriptors during background sync) Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D17931

DrahtBot added the CI failed label Mar 11, 2024

jrakibi closed this Mar 11, 2024

jrakibi reopened this Mar 11, 2024

BrandonOdiwuor reviewed Mar 11, 2024

View reviewed changes

test/functional/feature_assumeutxo.py Outdated Show resolved Hide resolved

ismaelsadeeq reviewed Mar 11, 2024

View reviewed changes

jrakibi changed the title ~~Add test for invalid UTXO snapshot with coin height above base height~~ test: Add test for invalid UTXO snapshot with coin height above base height Mar 11, 2024

jrakibi force-pushed the 2024/03/assumeUTXO-tests branch from 46f16a3 to 4104d9f Compare March 11, 2024 17:35

DrahtBot mentioned this pull request Mar 11, 2024

test: Assumeutxo: snapshots with less work should not be loaded #29428

Merged

DrahtBot added Tests and removed CI failed labels Mar 12, 2024

jrakibi force-pushed the 2024/03/assumeUTXO-tests branch from 885be24 to 0ca552c Compare March 13, 2024 00:00

DrahtBot added the CI failed label Mar 13, 2024

jrakibi force-pushed the 2024/03/assumeUTXO-tests branch 2 times, most recently from 21825ad to 28ab34d Compare March 13, 2024 00:36

jrakibi closed this Mar 13, 2024

jrakibi reopened this Mar 13, 2024

jrakibi force-pushed the 2024/03/assumeUTXO-tests branch from 28ab34d to a94cfca Compare March 13, 2024 00:50

This was referenced Mar 13, 2024

rpc: Optimize serialization and enhance metadata of dumptxoutset output #29612

Merged

assumeutxo: Get rid of faked nTx and nChainTx values #29370

Merged

DrahtBot removed the CI failed label Mar 13, 2024

fjahr mentioned this pull request Mar 14, 2024

AssumeUTXO Mainnet Readiness Tracking #29616

Closed

32 tasks

jrakibi force-pushed the 2024/03/assumeUTXO-tests branch from a94cfca to 1349ea9 Compare March 19, 2024 00:32

jrakibi changed the title ~~test: Add test for invalid UTXO snapshot with coin height above base height~~ test: Validate UTXO snapshot with coin height > base height & amount > money supply Mar 19, 2024

jrakibi changed the title ~~test: Validate UTXO snapshot with coin height > base height & amount > money supply~~ test: Validate UTXO snapshot with coin height > base height & amount > MAX_MONEY supply Mar 19, 2024

DrahtBot added the Needs rebase label Mar 20, 2024

achow101 requested a review from fjahr April 9, 2024 14:27

jrakibi force-pushed the 2024/03/assumeUTXO-tests branch from 1349ea9 to e807838 Compare April 10, 2024 13:54

DrahtBot removed the Needs rebase label Apr 10, 2024

DrahtBot mentioned this pull request Apr 11, 2024

contrib: add tool to convert compact-serialized UTXO set to SQLite database #27432

Merged

rkrux approved these changes Apr 15, 2024

View reviewed changes

test/functional/feature_assumeutxo.py Outdated Show resolved Hide resolved

fjahr reviewed Apr 15, 2024

View reviewed changes

test/functional/feature_assumeutxo.py Outdated Show resolved Hide resolved

test/functional/feature_assumeutxo.py Outdated Show resolved Hide resolved

test/functional/feature_assumeutxo.py Outdated Show resolved Hide resolved

jrakibi force-pushed the 2024/03/assumeUTXO-tests branch 3 times, most recently from f614e0e to 6428b36 Compare April 22, 2024 13:34

DrahtBot added the CI failed label Apr 22, 2024

fjahr reviewed Apr 22, 2024

View reviewed changes

DrahtBot requested a review from rkrux April 22, 2024 16:53

jrakibi force-pushed the 2024/03/assumeUTXO-tests branch 2 times, most recently from 8766cbb to ef27f36 Compare April 22, 2024 22:20

jrakibi force-pushed the 2024/03/assumeUTXO-tests branch from ef27f36 to ec1f1ab Compare April 22, 2024 22:37

DrahtBot removed the CI failed label Apr 23, 2024

achow101 merged commit 62ef33a into bitcoin:master May 2, 2024

bitcoin locked and limited conversation to collaborators May 2, 2025

	- TODO: Valid hash but invalid snapshot file (bad coin height or
	bad other serialization)

	"Testing snapshot file with an incorrect coin code: height 364 and coinbase 0"
	"Testing snapshot file with an incorrect coin height 364 and coinbase 0"

test: Validate UTXO snapshot with coin height > base height & amount > MAX_MONEY supply #29617

test: Validate UTXO snapshot with coin height > base height & amount > MAX_MONEY supply #29617

Uh oh!

Conversation

jrakibi commented Mar 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ensure snapshot loading fails for coins exceeding base height

Uh oh!

DrahtBot commented Mar 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

DrahtBot commented Mar 11, 2024

Uh oh!

BrandonOdiwuor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ismaelsadeeq left a comment

Choose a reason for hiding this comment

Uh oh!

ismaelsadeeq Mar 11, 2024

Choose a reason for hiding this comment

Uh oh!

jrakibi Mar 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

maflcko commented Mar 11, 2024

Uh oh!

DrahtBot commented Mar 13, 2024

Uh oh!

jrakibi commented Mar 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jrakibi commented Mar 19, 2024

Uh oh!

rkrux left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fjahr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jrakibi commented Apr 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fjahr left a comment

Choose a reason for hiding this comment

Uh oh!

fjahr Apr 22, 2024

Choose a reason for hiding this comment

Uh oh!

jrakibi Apr 22, 2024

Choose a reason for hiding this comment

Uh oh!

fjahr commented Apr 22, 2024

Uh oh!

maflcko commented Apr 23, 2024

Uh oh!

achow101 commented May 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jrakibi commented Mar 11, 2024 •

edited

Loading

DrahtBot commented Mar 11, 2024 •

edited

Loading

jrakibi Mar 11, 2024 •

edited

Loading

jrakibi commented Mar 13, 2024 •

edited

Loading

jrakibi commented Apr 22, 2024 •

edited

Loading