[avalanche] Send proof inventories
ClosedPublic
Actions

Authored by Fabien on Jun 8 2021, 15:19.

Details

Reviewers

PiRK

Group Reviewers

Restricted Project

Maniphest Tasks

Restricted Maniphest Task

Commits

rABC5794e8338bb8: [avalanche] Send proof inventories

Summary

Add the proof inventories in the inv messages. For now the proofs are
only added manually via the sendavalancheproof RPC.
This is heavily based on the current implementation for transactions.

Depends on D9654.

Supersedes D9489 and D9493.

Ref T1611.

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Branch

avalanche_send_inv

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 15947
Build 31794: Build Diff	lint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang
Build 31793: arc lint + arc unit

Event Timeline

Fabien created this revision.Jun 8 2021, 15:19

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 8 2021, 15:19

Fabien requested review of this revision.Jun 8 2021, 15:19

Note to reviewers:
This supersedes D9489 and D9493. I deliberately chose to not split apart the filtering of the peer's known proof (was D9493) because in the end I don't think it makes the review easier, but quite the opposite. By keeping it together with the inv message sending it makes it easier to compare with what is done for the proofs invs.
The broadcasting feature detailed in D9489 will be added in another diff.

Harbormaster completed remote builds in B15943: Diff 28810.Jun 8 2021, 15:35

Rebase

Harbormaster completed remote builds in B15947: Diff 28823.Jun 9 2021, 08:28

PiRK added a subscriber: PiRK.Jun 9 2021, 09:49

PiRK added inline comments.

src/avalanche/processor.cpp
502	I'm not sure about this. We just send the inventory, but there is no confirmation that the node received the message successfully. For transactions, we add them to the filter of known proofs when we receive an inventory message, but not when we send an inventory message.

PiRK added inline comments.Jun 9 2021, 10:01

src/avalanche/processor.cpp
502	Nevermind, my reasoning was backwards. A transaction can only be requested after we know it has been announced, so it makes senses to add it to the filter before the peers send the GETDATA request.

I should probably manage the loop delay first before enabling all this, will update/rebase once done

src/avalanche/processor.cpp
502	It's done the same for txs but the filter is filled directly with no call to AddKnownTx which makes it difficult to find: https://reviews.bitcoinabc.org/source/bitcoin-abc/browse/master/src/net_processing.cpp$5452