Skip to content

relay broadcast transactions to peers #1026

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
josibake wants to merge 1 commit into
base: master
Choose a base branch
from
+32 −2
Open

relay broadcast transactions to peers #1026

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions include/bitcoin/node/protocols/protocol_transaction_out_106.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,17 @@ class BCN_API protocol_transaction_out_106
/// Process tx announcement.
virtual bool do_announce(transaction_t link) NOEXCEPT;

virtual bool handle_broadcast_transaction(const code& ec,
const network::messages::peer::transaction::cptr& message,
uint64_t sender) NOEXCEPT;

virtual bool handle_receive_get_data(const code& ec,
const network::messages::peer::get_data::cptr& message) NOEXCEPT;
virtual void send_transaction(const code& ec, size_t index,
const network::messages::peer::get_data::cptr& message) NOEXCEPT;

virtual bool announce(const system::hash_digest& hash) NOEXCEPT;

private:
// These are thread safe.
const bool node_witness_;
Expand Down
28 changes: 26 additions & 2 deletions src/protocols/protocol_transaction_out_106.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ void protocol_transaction_out_106::start() NOEXCEPT

// Events subscription is asynchronous, events may be missed.
subscribe_chase(BIND(handle_chase, _1, _2, _3));
SUBSCRIBE_BROADCAST(transaction, handle_broadcast_transaction, _1, _2, _3);
SUBSCRIBE_CHANNEL(get_data, handle_receive_get_data, _1, _2);
protocol_peer::start();
}
Expand All @@ -54,6 +55,7 @@ void protocol_transaction_out_106::stopping(const code& ec) NOEXCEPT
// Unsubscriber race is ok.
BC_ASSERT(stranded());
unsubscribe_chase();
UNSUBSCRIBE_BROADCAST();
protocol_peer::stopping(ec);
}

Expand Down Expand Up @@ -98,8 +100,30 @@ bool protocol_transaction_out_106::do_announce(transaction_t link) NOEXCEPT
if (stopped())
return false;

// Don't announce to peer that announced to us.
const auto hash = query.get_tx_key(link);
return announce(query.get_tx_key(link));
}

bool protocol_transaction_out_106::handle_broadcast_transaction(const code& ec,
const transaction::cptr& message, uint64_t sender) NOEXCEPT
{
BC_ASSERT(stranded());

if (stopped(ec))
return false;

if (sender == identifier())
return true;

if (!message || !message->transaction_ptr)
Copy link
Copy Markdown
Member

@evoskuil evoskuil Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constitutes fault suppression, as it implies a contract failure that remains unexposed. This could be an assertion, as it should abort the process, and the assertion makes the failure easier to identify and documents the intent. However without the assertion the failure will occur as well.

This is such a common condition we don’t generally bother to assert for it. While some would consider the suppression good defensive programming, really it just condones a bug somewhere else. If this was public API surface the approach would be different, but this is internal communication under contract.

return true;

return announce(message->transaction_ptr->hash(false));
}

bool protocol_transaction_out_106::announce(const hash_digest& hash) NOEXCEPT
{
BC_ASSERT(stranded());

if (was_announced(hash))
return true;

Expand Down
Loading