Page MenuHomePhabricator
Differential D10523

[avalanche] Use the ProofPool multi index to store the orphan proofs
ClosedPublic
Actions

Authored by Fabien on Nov 24 2021, 16:42.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Maniphest Tasks
Restricted Maniphest Task
Commits
rABC527b5d07b687: [avalanche] Use the ProofPool multi index to store the orphan proofs
Summary

This replaces the orphan pool, and reuse the same structure and wrappers to store the orphans proofs mapped by utxo and id. The only change in behavior is that we don't allow orphans with conflicting utxos anymore, which requires an update in a functional test and a new unit test case.

Ref T1854.

Depends on D10597.

Test Plan
ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien created this revision.Nov 24 2021, 16:42
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 24 2021, 16:42
Fabien requested review of this revision.Nov 24 2021, 16:42
Fabien added a child revision: D10524: [avalanche] Select the favorite orphan in case of a conflict.Nov 24 2021, 16:48
Harbormaster completed remote builds in B17336: Diff 30988.Nov 24 2021, 16:53
Fabien planned changes to this revision.Nov 26 2021, 23:09

Need rebase after D10522 is updated

Fabien updated this revision to Diff 31138.Nov 29 2021, 13:23
Fabien edited the summary of this revision. (Show Details)
Fabien added a parent revision: D10586: [avalanche] Add a ProofPool getProof accessor to retrieve a proof from its id.

Rebase on top of D10586.

Harbormaster completed remote builds in B17437: Diff 31138.Nov 29 2021, 14:17
deadalnix requested changes to this revision.Nov 29 2021, 22:15
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/avalanche/peermanager.cpp
224 ↗(On Diff #31138)

Considering the class is here to maintain invariant about the pool, it shouldn't be public or manipulated in this way.

251 ↗(On Diff #31138)

The !! here ask the question of precedence, it realy isn't an improvement.

This revision now requires changes to proceed.Nov 29 2021, 22:15
Fabien updated this revision to Diff 31167.Nov 30 2021, 10:47

Adress feedback:

  • Use the rescan method instead of accessing the pool internals (rebased on top of D10598)
  • Revert the ambiguous !! to != nullptr

I also removed the previous orphan pool files that I forgot during the last diff update.

Fabien edited the summary of this revision. (Show Details)Nov 30 2021, 10:47
Fabien added a parent revision: D10598: [avalanche] Add a rescan method to the ProofPool.
Fabien removed parent revisions: D10586: [avalanche] Add a ProofPool getProof accessor to retrieve a proof from its id, D10522: [avalanche] Add wrappers to add and remove a proof from a proof pool.
Harbormaster completed remote builds in B17459: Diff 31167.Nov 30 2021, 10:49
teamcity added a comment.Nov 30 2021, 10:49
Build Bitcoin ABC Diffs / Diff Testing (lint-circular-dependencies) failed.

Tail of the build log:

/work /work/abc-ci-builds/lint-circular-dependencies
Good job! The circular dependency "avalanche/orphanproofpool -> avalanche/peermanager -> avalanche/orphanproofpool" is no longer present.
Please remove it from EXPECTED_CIRCULAR_DEPENDENCIES in /work/test/lint/lint-circular-dependencies.sh
to make sure this circular dependency is not accidentally reintroduced.

/work/abc-ci-builds/lint-circular-dependencies
Build lint-circular-dependencies failed with exit code 1
Fabien updated this revision to Diff 31168.Nov 30 2021, 10:51

Remove the old circular dependency

Harbormaster completed remote builds in B17460: Diff 31168.Nov 30 2021, 11:05
teamcity added a comment.Nov 30 2021, 11:05
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Tail of the build log:

rpc_signmessage.py                      | ✓ Passed  | 1 s
rpc_signrawtransaction.py               | ✓ Passed  | 3 s
rpc_txoutproof.py                       | ✓ Passed  | 1 s
rpc_uptime.py                           | ✓ Passed  | 1 s
rpc_users.py                            | ✓ Passed  | 6 s
rpc_whitelist.py                        | ✓ Passed  | 1 s
tool_wallet.py                          | ✓ Passed  | 6 s
wallet_abandonconflict.py               | ✓ Passed  | 7 s
wallet_address_types.py                 | ✓ Passed  | 15 s
wallet_avoidreuse.py                    | ✓ Passed  | 5 s
wallet_avoidreuse.py --descriptors      | ✓ Passed  | 7 s
wallet_backup.py                        | ✓ Passed  | 25 s
wallet_balance.py                       | ✓ Passed  | 11 s
wallet_basic.py                         | ✓ Passed  | 23 s
wallet_coinbase_category.py             | ✓ Passed  | 1 s
wallet_create_tx.py                     | ✓ Passed  | 8 s
wallet_createwallet.py                  | ✓ Passed  | 3 s
wallet_createwallet.py --usecli         | ✓ Passed  | 3 s
wallet_descriptor.py                    | ✓ Passed  | 18 s
wallet_disable.py                       | ✓ Passed  | 1 s
wallet_dump.py                          | ✓ Passed  | 5 s
wallet_encryption.py                    | ✓ Passed  | 5 s
wallet_encryption.py --descriptors      | ✓ Passed  | 5 s
wallet_hd.py                            | ✓ Passed  | 6 s
wallet_hd.py --descriptors              | ✓ Passed  | 6 s
wallet_import_rescan.py                 | ✓ Passed  | 6 s
wallet_import_with_label.py             | ✓ Passed  | 1 s
wallet_importdescriptors.py             | ✓ Passed  | 8 s
wallet_importmulti.py                   | ✓ Passed  | 3 s
wallet_importprunedfunds.py             | ✓ Passed  | 3 s
wallet_keypool.py                       | ✓ Passed  | 3 s
wallet_keypool_topup.py                 | ✓ Passed  | 3 s
wallet_keypool_topup.py --descriptors   | ✓ Passed  | 5 s
wallet_labels.py                        | ✓ Passed  | 2 s
wallet_labels.py --descriptors          | ✓ Passed  | 5 s
wallet_listreceivedby.py                | ✓ Passed  | 19 s
wallet_listsinceblock.py                | ✓ Passed  | 4 s
wallet_listtransactions.py              | ✓ Passed  | 9 s
wallet_multiwallet.py --usecli          | ✓ Passed  | 13 s
wallet_reorgsrestore.py                 | ✓ Passed  | 4 s
wallet_resendwallettransactions.py      | ✓ Passed  | 2 s
wallet_send.py                          | ✓ Passed  | 8 s
wallet_startup.py                       | ✓ Passed  | 4 s
wallet_txn_clone.py                     | ✓ Passed  | 3 s
wallet_txn_clone.py --mineblock         | ✓ Passed  | 7 s
wallet_txn_doublespend.py               | ✓ Passed  | 4 s
wallet_txn_doublespend.py --mineblock   | ✓ Passed  | 5 s
wallet_watchonly.py                     | ✓ Passed  | 1 s
wallet_watchonly.py --usecli            | ✓ Passed  | 5 s

ALL                                     | ✓ Passed  | 1049 s (accumulated) 
Runtime: 211 s

----------------------------------------------------------------------
Ran 10 tests in 0.093s

OK

ninja: build stopped: cannot make progress due to previous errors.
Build build-diff failed with exit code 1
Fabien added a comment.Nov 30 2021, 12:48

Unrelated failure, restarted the build

deadalnix accepted this revision.Nov 30 2021, 17:00
deadalnix added inline comments.
src/avalanche/peermanager.cpp
238 ↗(On Diff #31168)

I don't really think this is an improvement.

This revision is now accepted and ready to land.Nov 30 2021, 17:00
Fabien updated this revision to Diff 31184.Nov 30 2021, 19:54

Rebase on top of D10597, fix style as per comment

Fabien edited the summary of this revision. (Show Details)Nov 30 2021, 19:55
Fabien added a parent revision: D10597: [avalanche] Make the proof pool internal container private.
Fabien removed a parent revision: D10598: [avalanche] Add a rescan method to the ProofPool.
Harbormaster completed remote builds in B17468: Diff 31184.Nov 30 2021, 20:14
Fabien updated this revision to Diff 31209.Dec 1 2021, 15:03

Rebase

Harbormaster completed remote builds in B17491: Diff 31209.Dec 1 2021, 15:22
Closed by commit rABC527b5d07b687: [avalanche] Use the ProofPool multi index to store the orphan proofs (authored by Fabien). · Explain WhyDec 5 2021, 10:15
This revision was automatically updated to reflect the committed changes.
Fabien added a commit: rABC527b5d07b687: [avalanche] Use the ProofPool multi index to store the orphan proofs.
Fabien removed a child revision: D10524: [avalanche] Select the favorite orphan in case of a conflict.Dec 9 2021, 16:56

Revision Contents
Changeset List

Diff 31269

View Options

src/CMakeLists.txt

Loading...
View Options

src/avalanche/orphanproofpool.h

Loading...
View Options

src/avalanche/orphanproofpool.cpp

Loading...
View Options

src/avalanche/peermanager.h

Loading...
View Options

src/avalanche/peermanager.cpp

Loading...
View Options

src/avalanche/test/CMakeLists.txt

Loading...
View Options

src/avalanche/test/orphanproofpool_tests.cpp

Loading...
View Options

src/avalanche/test/peermanager_tests.cpp

Loading...
View Options

test/functional/abc_rpc_avalancheproof.py

Loading...
View Options

test/lint/lint-circular-dependencies.sh

Loading...