Differential D12999

[avalanche] Sync proofs when functional tests call sync_all
ClosedPublic
Actions

Authored by sdulfari on Jan 18 2023, 17:27.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC0b22e71ce430: [avalanche] Sync proofs when functional tests call sync_all

Summary

Now that avalanche is enabled by default the test framework can treat it as
such. It is natural to expect sync_all to sync all inventories, including
proofs.

Test Plan

ninja check-functional

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

sdulfari created this revision.Jan 18 2023, 17:27

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 18 2023, 17:27

sdulfari requested review of this revision.Jan 18 2023, 17:27

sdulfari added inline comments.Jan 18 2023, 17:30

test/functional/abc_p2p_avalanche_voting.py
40 ↗	(On Diff #37582)	The alternate approach is to sync_blocks on the two generate() calls that dont have sync_fun set in this test, but that gives a bad code smell because there is no explicit reason to not sync the proofs. The test simply does not rely on them. It is more correct to have the node configs match so that they sync proofs without modifying the test.

Harbormaster completed remote builds in B21782: Diff 37582.Jan 18 2023, 17:40

Fabien requested changes to this revision.Jan 19 2023, 09:28

Fabien added a subscriber: Fabien.

Fabien added inline comments.

test/functional/abc_p2p_proof_inventory.py
200 ↗	(On Diff #37582)	Something is wrong here. You synced all the nodes, but this test expected the last one to not sync the proofs
test/functional/abc_rpc_avalancheproof.py
193 ↗	(On Diff #37582)	why ?

This revision now requires changes to proceed.Jan 19 2023, 09:28

sdulfari requested review of this revision.Jan 20 2023, 16:53

sdulfari added inline comments.

test/functional/abc_p2p_proof_inventory.py
200 ↗	(On Diff #37582)	Yes, which is why I introduced sync_fun=self.sync_blocks so that proofs are not explicitly synced between all nodes.
test/functional/abc_rpc_avalancheproof.py
193 ↗	(On Diff #37582)	The node is started with an immature proof, so while the proof may be sent to peered nodes, sync_proofs only checks getavalanchepeerinfo which shows ava peers. Until the proof matures, it will not appear here.