Skip to content

Should correctly broadcast light transactions#9448

Merged
LukaszRozmej merged 7 commits intomasterfrom
fix/light-transaction-broadcast-proof-version
Oct 13, 2025
Merged

Should correctly broadcast light transactions#9448
LukaszRozmej merged 7 commits intomasterfrom
fix/light-transaction-broadcast-proof-version

Conversation

@LukaszRozmej
Copy link
Copy Markdown
Member

Changes

  • Correctly use GetProofVersion for light transactions to determine broadcasting.

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)

Testing

Requires testing

  • Yes

If yes, did you write tests?

  • Yes

Copy link
Copy Markdown
Contributor

@marcindsobczak marcindsobczak left a comment

Choose a reason for hiding this comment

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

There is no bug in production code, as we call SpecDrivenTxGossipPolicy.ShouldGossipTransaction() only with full blob transaction as an argument.
But I agree that current code is a bit misleading and it is worth to refactor it. After changes from this PR it is cleaner. There will be no change in actual behavior, but code will bo more robust

Comment thread src/Nethermind/Nethermind.TxPool.Test/TxBroadcasterTests.cs
Comment thread src/Nethermind/Nethermind.TxPool.Test/TxBroadcasterTests.cs Outdated
@marcindsobczak
Copy link
Copy Markdown
Contributor

This check is actually preventing us from announcing blob txs to the newly connected peer and from reannouncing after new block arrival. So it is not only refactor, it is a bug fix.

@LukaszRozmej LukaszRozmej marked this pull request as ready for review October 13, 2025 10:24
public virtual ProofVersion? GetProofVersion() => SupportsBlobs
? this is { NetworkWrapper: ShardBlobNetworkWrapper { Version: var version } }
? version
: ProofVersion.V0
Copy link
Copy Markdown
Member Author

@LukaszRozmej LukaszRozmej Oct 13, 2025

Choose a reason for hiding this comment

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

should this return null if no wrapper?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

or throw an exception if no wrapper but proof version is requested?

@LukaszRozmej LukaszRozmej merged commit 3bb883e into master Oct 13, 2025
80 checks passed
@LukaszRozmej LukaszRozmej deleted the fix/light-transaction-broadcast-proof-version branch October 13, 2025 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants