Differential D9493

[avalanche] Prevent sending proof invs multiple times to the same peer
AbandonedPublic
Actions

Authored by Fabien on May 10 2021, 14:33.

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Maniphest Tasks

Restricted Maniphest Task

Summary

This prevents from spamming a peer with the same proof inventory. After
an inventory has been sent to a peer it is added to a rolling bloom
filter. This filter is then checked for existing proof id before filling
the proof inventory send buffer.

This is of limited use at this stage but will become more relevant when
we start relaying the proofs from the network.

Depends on D9492.

Test Plan

./test/functional/test_runner.py abc_p2p_proof_inventory

Diff Detail

Repository

rABC Bitcoin ABC

Branch

avalanche_filter_known_invs

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 15671
Build 31246: Build Diff	lint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang
Build 31245: arc lint + arc unit

Event Timeline

Fabien created this revision.May 10 2021, 14:33

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 10 2021, 14:33

Fabien requested review of this revision.May 10 2021, 14:33

Harbormaster completed remote builds in B15671: Diff 28404.May 10 2021, 14:43

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

Failed tests logs:

====== Bitcoin ABC functional tests: abc_rpc_avalancheproof.py ======

------- Stdout: -------
2021-05-10T14:42:28.118000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210510_144143/abc_rpc_avalancheproof_597
2021-05-10T14:42:28.410000Z TestFramework (INFO): Make build a valid proof and restart the node to use it
2021-05-10T14:42:28.819000Z TestFramework (INFO): The proof verification should be delayed until IBD is complete
2021-05-10T14:42:28.822000Z TestFramework (INFO): Generate delegations for the proof
2021-05-10T14:42:28.859000Z TestFramework (INFO): Check the sendavalancheproof RPC
2021-05-10T14:42:29.215000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 126, in main
    self.run_test()
  File "/work/test/functional/abc_rpc_avalancheproof.py", line 221, in run_test
    check_sendavalancheproof_failure(too_many_utxos, "too-many-utxos")
UnboundLocalError: local variable 'too_many_utxos' referenced before assignment
2021-05-10T14:42:29.266000Z TestFramework (INFO): Stopping nodes
2021-05-10T14:42:29.417000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210510_144143/abc_rpc_avalancheproof_597
2021-05-10T14:42:29.417000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210510_144143/abc_rpc_avalancheproof_597/test_framework.log
2021-05-10T14:42:29.417000Z TestFramework (ERROR): 
2021-05-10T14:42:29.417000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-without-wallet/test/tmp/test_runner_₿₵_  _20210510_144143/abc_rpc_avalancheproof_597' to consolidate all logs
2021-05-10T14:42:29.417000Z TestFramework (ERROR): 
2021-05-10T14:42:29.417000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2021-05-10T14:42:29.417000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2021-05-10T14:42:29.417000Z TestFramework (ERROR):

Each failure log is accessible here:
Bitcoin ABC functional tests: abc_rpc_avalancheproof.py

Fabien planned changes to this revision.May 10 2021, 14:45

Rebase to fix the build with wallet disabled

Harbormaster completed remote builds in B15673: Diff 28406.May 10 2021, 15:14

deadalnix requested changes to this revision.May 10 2021, 15:43

deadalnix added a subscriber: deadalnix.

deadalnix added inline comments.

src/net.h
981 ↗	(On Diff #28406)	GUARDED_BY(cs_tx_inventory) No?
1179 ↗	(On Diff #28406)	I'm nto sure this WITH8LOCK construct really is more readable than the alternative, but it's okay either way.
src/net_processing.cpp
5379 ↗	(On Diff #28406)	I don't follow why this is needed. I don't think it is.