Verify the peer's signature using his delegated pubkey.
Add functional tests for the AvaHello handshake sequence.
Add a missing return at the end of the AvaHello branch in net_processing.cpp
Depends on D9564
Details
Details
- Reviewers
Fabien - Group Reviewers
Restricted Project - Commits
- rABC819d42939969: [avalanche] verify signature on avahello reception
ninja check-functional
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 15838 Build 31576: Build Diff lint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang Build 31575: arc lint + arc unit
Event Timeline
| src/net_processing.cpp | ||
|---|---|---|
| 4025 ↗ | (On Diff #28562) | Out of scope here but at some point we might want a dedicated BCLog::AVALANCHE category |
| test/functional/abc_p2p_avalanche.py | ||
| 119 ↗ | (On Diff #28562) | Nit: if this is only use in send_avahello then you can define it there and avoid the leading _ which is not consistent with what is done in the codebase |
| 412 ↗ | (On Diff #28562) | Style nit: you can use interface.wait_for_disconnect() to test the ban |
Comment Actions
rebase
Simplify TestNode.sendHello. Use waitfordisconnect to check banning behavior (reduces the test fragility if the logging changes)
| src/net_processing.cpp | ||
|---|---|---|
| 3939 ↗ | (On Diff #28612) | This might become very spammy, no? In any case, this doesn't really help with anything because it doesn't have the relevant information to figure out what is going on. |
Comment Actions
remove logging on successfull signature verification. This makes it a difficult to test the success in the functional test. For the time being, checking that the good p2p interface is still connected after the bad one is disconnected is better than no test at all.
Comment Actions
Improve the check that the good interface is still connected, using sync_with_ping. This raises AssertionError if no pong is received in response to ping.
| test/functional/abc_p2p_avalanche.py | ||
|---|---|---|
| 403 | Please do both: check the node is disconnected AND the reason it is disconnected. Otherwise you're not sure what you are testing, here you are looking for invalid-avahello-signature but you if you get an invalid-delegation the test will still pass. | |
| 408 | Move next to the good interface test instead of having it dandling | |
Comment Actions
address review:
- move sync_and_ping next to the corresponding code
- add test for log message again to verify the reason the node is banned for