Skip to content

Conversation

alfonsoromanz
Copy link
Contributor

I am getting familiar with the assume_utxo tests and I found that the scenario of trying to activate a snapshot twice is not covered. This test is to ensure failure when loading a snapshot if there is already a snapshot-based chainstate.

@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 26, 2024

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, rkrux, kevkevinpal, achow101

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

@fjahr
Copy link
Contributor

fjahr commented Apr 26, 2024

test/functional/feature_assumeutxo.py:401:1: W293 blank line contains whitespace
^---- failure generated from lint-python.py

@fjahr
Copy link
Contributor

fjahr commented Apr 26, 2024

Concept ACK

@fjahr fjahr mentioned this pull request Apr 26, 2024
32 tasks
@alfonsoromanz alfonsoromanz force-pushed the assumeutxo-test-import-snapshot-twice branch from 9b75541 to b259b0e Compare April 26, 2024 13:50
@DrahtBot
Copy link
Contributor

🚧 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/24300439204

@alfonsoromanz
Copy link
Contributor Author

alfonsoromanz commented Apr 26, 2024

test/functional/feature_assumeutxo.py:401:1: W293 blank line contains whitespace
^---- failure generated from lint-python.py

Thanks @fjahr. I just fixed the linting issue.

@fjahr
Copy link
Contributor

fjahr commented Apr 26, 2024

Code review ACK b259b0e

@alfonsoromanz alfonsoromanz changed the title [Test] Assumeutxo: ensure failure when importing a snapshot twice test: Assumeutxo: ensure failure when importing a snapshot twice Apr 26, 2024
@DrahtBot DrahtBot added Tests and removed CI failed labels Apr 26, 2024
Copy link
Contributor

@rkrux rkrux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK b259b0e

Make successful, all functional tests pass.
Short and to the point PR.

@kevkevinpal
Copy link
Contributor

tACK b259b0e

made a similar PR not knowing this existed #30068

@achow101
Copy link
Member

achow101 commented May 9, 2024

ACK b259b0e

@achow101 achow101 merged commit 921c61e into bitcoin:master May 9, 2024
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Apr 17, 2025
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 <sebastian.falbesoner@gmail.com>

bitcoin/bitcoin@ce585a9
[[bitcoin/bitcoin#28652 | core#28652]]
[[bitcoin/bitcoin#29345 | core#29345]]

------

rpc: add getchainstates

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

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
@bitcoin bitcoin locked and limited conversation to collaborators May 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants