[avalanche] Improve the conflicting proof selection algorithm
ClosedPublic
Actions

Authored by Fabien on Nov 5 2021, 11:49.

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Maniphest Tasks

Restricted Maniphest Task

Commits

rABC7df196b92feb: [avalanche] Improve the conflicting proof selection algorithm

Summary

The current algorithm special cases conflicting proofs using the same master key, so that it is possible to update a proof with a lower amount by increasing the sequence number.

But this comparator is incomplete: if 2 conflicting proofs with the same sequence number are submitted, the last arrived one is rejected. This comparaison exhibit a weird behavior: it is possible to get isConflictingProofPreferred(a, b) == isConflictingProofPreferred(b, a). For example, this prevents from using this algo for sorting.

This diff changes the behavior so that identical sequence are compared by the other parameters, i.e. amount, then stake count, then id. This fixes the above issue, while maintaining the possibility for a user to update a proof with a lower stake amount as long as he owns the master key.

As a consequence the unit test can be simplified in a single case to avoid duplication.

Ref T1854.

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 5 2021, 11:49

Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 5 2021, 11:49

Fabien requested review of this revision.Nov 5 2021, 11:49

Harbormaster completed remote builds in B17195: Diff 30789.Nov 5 2021, 12:03

Fabien added a child revision: D10439: [avalanche] Make `isConflictingProofPreferred` a comparator.Nov 8 2021, 08:09

Rebase

Harbormaster completed remote builds in B17215: Diff 30820.Nov 10 2021, 09:43

Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.

Tail of the build log:

[0m[0;32mwallet_encryption.py --descriptors      | ✓ Passed  | 5 s
[0m[0;32mwallet_hd.py                            | ✓ Passed  | 8 s
[0m[0;32mwallet_hd.py --descriptors              | ✓ Passed  | 6 s
[0m[0;32mwallet_import_rescan.py                 | ✓ Passed  | 10 s
[0m[0;32mwallet_import_with_label.py             | ✓ Passed  | 1 s
[0m[0;32mwallet_importdescriptors.py             | ✓ Passed  | 7 s
[0m[0;32mwallet_importmulti.py                   | ✓ Passed  | 6 s
[0m[0;32mwallet_importprunedfunds.py             | ✓ Passed  | 2 s
[0m[0;32mwallet_keypool.py                       | ✓ Passed  | 3 s
[0m[0;32mwallet_keypool_topup.py                 | ✓ Passed  | 4 s
[0m[0;32mwallet_keypool_topup.py --descriptors   | ✓ Passed  | 4 s
[0m[0;32mwallet_labels.py                        | ✓ Passed  | 2 s
[0m[0;32mwallet_labels.py --descriptors          | ✓ Passed  | 2 s
[0m[0;32mwallet_listreceivedby.py                | ✓ Passed  | 14 s
[0m[0;32mwallet_listsinceblock.py                | ✓ Passed  | 6 s
[0m[0;32mwallet_listtransactions.py              | ✓ Passed  | 11 s
[0m[0;32mwallet_multiwallet.py --usecli          | ✓ Passed  | 17 s
[0m[0;32mwallet_reorgsrestore.py                 | ✓ Passed  | 4 s
[0m[0;32mwallet_resendwallettransactions.py      | ✓ Passed  | 3 s
[0m[0;32mwallet_send.py                          | ✓ Passed  | 8 s
[0m[0;32mwallet_startup.py                       | ✓ Passed  | 3 s
[0m[0;32mwallet_txn_clone.py                     | ✓ Passed  | 3 s
[0m[0;32mwallet_txn_clone.py --mineblock         | ✓ Passed  | 4 s
[0m[0;32mwallet_txn_doublespend.py               | ✓ Passed  | 3 s
[0m[0;32mwallet_txn_doublespend.py --mineblock   | ✓ Passed  | 4 s
[0m[0;32mwallet_watchonly.py                     | ✓ Passed  | 1 s
[0m[0;32mwallet_watchonly.py --usecli            | ✓ Passed  | 2 s
[0m[1m
ALL                                     | ✓ Passed  | 1275 s (accumulated) 
[0mRuntime: 256 s

----------------------------------------------------------------------
Ran 10 tests in 0.162s

OK

[197/456] Running avalanche test suite
PASSED: avalanche test suite
[198/456] Running seeder test suite
PASSED: seeder test suite
[199/456] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.004s

OK
[200/456] cd /work/contrib/devtools/chainparams && /usr/bin/python3.7 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[426/456] Running pow test suite
PASSED: pow test suite
[449/456] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[454/456] Running bitcoin test suite
PASSED: bitcoin test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-debug failed with exit code 1

Unrelated failure, restarting the build

deadalnix accepted this revision.Nov 17 2021, 17:38

This revision is now accepted and ready to land.Nov 17 2021, 17:38

Closed by commit rABC7df196b92feb: [avalanche] Improve the conflicting proof selection algorithm (authored by Fabien). · Explain WhyNov 18 2021, 08:24

This revision was automatically updated to reflect the committed changes.

Fabien added a commit: rABC7df196b92feb: [avalanche] Improve the conflicting proof selection algorithm.