Paths

Table of Contentst

Differential D11240

tests: add snapshot activation test
ClosedPublic
Actions

Authored by PiRK on Mar 22 2022, 15:14.

Tags

None

Subscribers

Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABC0e3eebdb3618: tests: add snapshot activation test

Summary

This also tests the hashes added in D11238.

This is a backport of core#19806 [6/8]
https://github.com/bitcoin/bitcoin/pull/19806/commits/4d8de04f32736199e4b41a14a2d29b1a4d0a15d4

Depends on D11239

Test Plan

ninja check

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

PiRK created this revision.Mar 22 2022, 15:14

Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 22 2022, 15:14

PiRK requested review of this revision.Mar 22 2022, 15:15

teamcity edited the summary of this revision. (Show Details)Mar 22 2022, 15:15

PiRK added a child revision: D11241: test: Add tests with maleated snapshot data.Mar 22 2022, 15:28

Harbormaster completed remote builds in B18594: Diff 32886.Mar 22 2022, 15:31

rebase

Harbormaster completed remote builds in B18609: Diff 32909.Mar 23 2022, 10:34

There are a few issues worth fixing together with this stack: https://github.com/bitcoin/bitcoin/pull/21584/
Please make sure to pull them along with the relevant features.

src/test/validation_chainstatemanager_tests.cpp
256 ↗	(On Diff #32909)	BlockHash
290 ↗	(On Diff #32909)	move comment
test/sanitizer_suppressions/tsan
31 ↗	(On Diff #32909)	Any idea where this comes from ?

This revision now requires changes to proceed.Mar 25 2022, 15:29

This is turning out more complex than I expected, and I think the single commit that was needed as a dependency is already landed (D11224)

test/sanitizer_suppressions/tsan
31 ↗	(On Diff #32909)	I assume it is related to the code accessing the coins_cache after releasing `cs_main`, which is supposed to be OK in this case. See for instance the comment here: https://github.com/bitcoin/bitcoin/pull/19806/files#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b5b98R5296 Similar things are done in the unit test, the lock is held only the time to acquire the coins cache and released immediately, because no one else use this CChainStateManager.

PiRK mentioned this in D11238: chainparams: add allowed assumeutxo values.Apr 11 2022, 12:29

rebase

Harbormaster completed remote builds in B18782: Diff 33153.Apr 11 2022, 15:46

Fabien requested changes to this revision.Apr 11 2022, 20:21

Fabien added inline comments.

src/test/validation_chainstatemanager_tests.cpp
237 ↗	(On Diff #33153)	This is still missing the changes from #21584 (should be in D11331 ?)

This revision now requires changes to proceed.Apr 11 2022, 20:21

PiRK mentioned this in D11239: simplify ChainstateManager::SnapshotBlockhash() return semantics.Apr 12 2022, 09:26

PiRK requested review of this revision.Apr 12 2022, 09:35

PiRK added inline comments.

src/test/validation_chainstatemanager_tests.cpp
237 ↗	(On Diff #33153)	I made sure D11331 depends on this (via D11241), to clarify the dependencies.

Fabien accepted this revision.Apr 12 2022, 20:43

This revision is now accepted and ready to land.Apr 12 2022, 20:43

Closed by commit rABC0e3eebdb3618: tests: add snapshot activation test (authored by James O'Beirne <james.obeirne@pm.me>, committed by PiRK). · Explain WhyApr 13 2022, 06:39

This revision was automatically updated to reflect the committed changes.

PiRK added a commit: rABC0e3eebdb3618: tests: add snapshot activation test.

Revision Contents
Changeset List

Path

Size

src/

test/

validation_chainstatemanager_tests.cpp

149 lines

test/

sanitizer_suppressions/

1 line

Diff 33187

src/test/validation_chainstatemanager_tests.cpp

Loading...

test/sanitizer_suppressions/tsan

Loading...