[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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

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 ↗	(On Diff #28823)	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 ↗	(On Diff #28823)	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 ↗	(On Diff #28823)	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