When adding/remove peers/nodes we track the scores so we can quickly query the state for quorum
detection.
Details
Details
- Reviewers
Fabien - Group Reviewers
Restricted Project - Maniphest Tasks
- Restricted Maniphest Task
- Commits
- rABCdf31b9e30915: [avalanche] Track registered and allocated scores in PeerManager.
ninja check-avalanche
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- TS_score_tracking
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 18503 Build 36802: Build Diff lint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang Build 36800: arc lint + arc unit
Event Timeline
Comment Actions
Mostly good, additions/subtractions/asserts are done in the right place.
The main problem is the name of the variables being confusing. The tests can be improved a bit (see suggestions).
| src/avalanche/peermanager.h | ||
|---|---|---|
| 204 | You don't register or allocate a score, do you ? | |
| src/avalanche/test/peermanager_tests.cpp | ||
| 1685 | Don't rely on peer id, retrieve it instead | |
| 1687 | You never checked that the scores sum up in the presence of several peers | |
| 1694 | uint32_t score1 = 10000000 * MIN_VALID_PROOF_SCORE; auto proof = buildRandomProof(score1); | |
| 1716 | Dito | |
| 1747 | You can also test that removing a peer with nodes reduce the allocated score as expected | |
| src/avalanche/peermanager.h | ||
|---|---|---|
| 203 ↗ | (On Diff #32778) | Let's try to align with the getavalancheinfo rpc as much as possible, since the names have already been discussed there totalPeersScore. |
| src/avalanche/test/peermanager_tests.cpp | ||
| 46 ↗ | (On Diff #32778) | I don't see the point of splitting this, there is no reason not to use the actual facility directly. |
| src/avalanche/test/peermanager_tests.cpp | ||
|---|---|---|
| 46 ↗ | (On Diff #32778) | The peerid changes when the peer is moved out of and back into the valid pool, so getting the peerid when it's first registered doesn't help. |
| src/avalanche/test/peermanager_tests.cpp | ||
|---|---|---|
| 46 ↗ | (On Diff #32778) | OK |