Page MenuHomePhabricator

[avalanche] Send proof inventories
ClosedPublic

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

Event Timeline

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.

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.

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.

Fabien planned changes to this revision.Jun 9 2021, 10:05

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

This revision is now accepted and ready to land.Jun 9 2021, 17:28
This revision was automatically updated to reflect the committed changes.