Page MenuHomePhabricator

[net processing] Move tx & proof relay data to Peer
ClosedPublic

Authored by PiRK on Oct 31 2023, 12:14.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCa3fde16a0c14: [net processing] Move tx & proof relay data to Peer
Summary

Partial backport of core#21160
https://github.com/bitcoin/bitcoin/pull/21160/commits/575bbd0dea6d12510fdf3220d0f0e47d969da6e9

Regarding the change in processor.cpp: AddKnownProof is now a static function in net_processing, so the call to add the local proof to the peer's known proofs has been moved there, just after SendHello.
The condition for calling this line of code is equivalent, the only possible change in behavior is that the proofid is no longer added to the known proofs if the schnorr signature fails in Processor::sendHelloInternal which would mean our sessionKey is invalid.

Depends on D14709

Test Plan

ninja all check-all bitcoin-fuzzers

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK requested review of this revision.Oct 31 2023, 12:14
PiRK edited the test plan for this revision. (Show Details)

make AddKnownProof static, add fuzzer compiling to test plan

Fabien requested changes to this revision.Oct 31 2023, 14:51
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/net_processing.cpp
2513 ↗(On Diff #42872)

It's interesting that there is no test breaking, this is a bug: if the peer don't want you to relay txs, you should continue to the next one and not bail out.
EDIT: checked on core, they're using continue

2528 ↗(On Diff #42872)

dito

3924 ↗(On Diff #42872)

OK

This revision now requires changes to proceed.Oct 31 2023, 14:51

fix RelayProof and RelayTransaction: don't bail out for the first peer that does not want txs relayed

This revision is now accepted and ready to land.Nov 1 2023, 08:58