Skip to content

Commit 3bb883e

Browse files
authored
Should correctly broadcast light transactions (#9448)
* Should correctly broadcast light transactions * Add all combinations to tests * Make GetProofVersion polimorphic and nullable * make it better * fix MempoolBlobTxProofVersionValidator * Return null on missing wrapper * fix
1 parent c5cfd08 commit 3bb883e

6 files changed

Lines changed: 51 additions & 21 deletions

File tree

src/Nethermind/Nethermind.Consensus/Validators/TxValidator.cs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -307,13 +307,12 @@ private MempoolBlobTxProofVersionValidator() { }
307307

308308
public ValidationResult IsWellFormed(Transaction transaction, IReleaseSpec releaseSpec)
309309
{
310-
return transaction switch
311-
{
312-
LightTransaction lightTx => ValidateProofVersion(lightTx.ProofVersion, releaseSpec),
313-
{ Type: TxType.Blob, NetworkWrapper: ShardBlobNetworkWrapper wrapper } => ValidateProofVersion(wrapper.Version, releaseSpec),
314-
{ Type: TxType.Blob, NetworkWrapper: not null } => TxErrorMessages.InvalidTransactionForm,
315-
_ => ValidationResult.Success,
316-
};
310+
if (!transaction.SupportsBlobs) return ValidationResult.Success;
311+
312+
ProofVersion? version = transaction.GetProofVersion();
313+
return version is null
314+
? transaction.NetworkWrapper is not null ? TxErrorMessages.InvalidTransactionForm : ValidationResult.Success
315+
: ValidateProofVersion(version.Value, releaseSpec);
317316

318317
static ValidationResult ValidateProofVersion(ProofVersion txProofVersion, IReleaseSpec spec) =>
319318
txProofVersion != spec.BlobProofVersion ? TxErrorMessages.InvalidProofVersion : ValidationResult.Success;

src/Nethermind/Nethermind.Core/Transaction.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -332,12 +332,17 @@ public void CopyTo(Transaction tx)
332332
tx._size = _size;
333333
tx.AuthorizationList = AuthorizationList;
334334
}
335+
336+
public virtual ProofVersion? GetProofVersion() =>
337+
SupportsBlobs && this is { NetworkWrapper: ShardBlobNetworkWrapper { Version: var version } }
338+
? version
339+
: null;
335340
}
336341

337342
/// <summary>
338343
/// Transaction that is generated by the node to be included in future block. After included in the block can be handled as regular <see cref="Transaction"/>.
339344
/// </summary>
340-
public sealed class GeneratedTransaction : Transaction { }
345+
public sealed class GeneratedTransaction : Transaction;
341346

342347
/// <summary>
343348
/// System transaction that is to be executed by the node without including in the block.
@@ -350,9 +355,7 @@ public sealed class SystemTransaction : Transaction
350355
/// <summary>
351356
/// System call like transaction that is to be executed by the node without including in the block.
352357
/// </summary>
353-
public sealed class SystemCall : Transaction
354-
{
355-
}
358+
public sealed class SystemCall : Transaction;
356359

357360
/// <summary>
358361
/// Used inside Transaction::GetSize to calculate encoded transaction size

src/Nethermind/Nethermind.TxPool.Test/TxBroadcasterTests.cs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Linq;
77
using System.Runtime.CompilerServices;
88
using System.Threading.Tasks;
9+
using FastEnumUtility;
910
using FluentAssertions;
1011
using Nethermind.Blockchain;
1112
using Nethermind.Consensus;
@@ -729,6 +730,38 @@ public void calculation_of_baseFeeThreshold_should_handle_overflow_correctly([Va
729730
: baseFeeThreshold);
730731
}
731732

733+
[Test]
734+
public void can_correctly_broadcast_light_transactions_without_wrappers([Values] ProofVersion proofVersion, [Values] bool versionMatches)
735+
{
736+
// Arrange
737+
IChainHeadInfoProvider mockChainHeadInfoProvider = Substitute.For<IChainHeadInfoProvider>();
738+
mockChainHeadInfoProvider.CurrentProofVersion.Returns(proofVersion);
739+
IReleaseSpec spec = Substitute.For<IReleaseSpec>();
740+
spec.BlobProofVersion.Returns(versionMatches ? proofVersion : GetInvalidVersion(proofVersion));
741+
742+
SpecDrivenTxGossipPolicy gossipPolicy = new(mockChainHeadInfoProvider);
743+
744+
Transaction blobTransaction = Build.A.Transaction
745+
.WithShardBlobTxTypeAndFields(spec: spec)
746+
.SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyA)
747+
.TestObject;
748+
749+
LightTransaction lightTransaction = new(blobTransaction);
750+
751+
// Act
752+
bool result = gossipPolicy.ShouldGossipTransaction(lightTransaction);
753+
754+
// Assert
755+
result.Should().Be(versionMatches, "LightTransaction from blob transaction should be gossiped when proof version matches.");
756+
757+
// Gets (version + 1) % (version + 1) - so next version round robin
758+
ProofVersion GetInvalidVersion(ProofVersion version)
759+
{
760+
byte mod = (byte)(FastEnum.GetMaxValue<ProofVersion>() + 1);
761+
return (ProofVersion)((byte)(version + 1) % mod);
762+
}
763+
}
764+
732765
private (IList<Transaction> expectedTxs, IList<Hash256> expectedHashes) GetTxsAndHashesExpectedToBroadcast(Transaction[] transactions, int expectedCountTotal)
733766
{
734767
List<Transaction> expectedTxs = new();

src/Nethermind/Nethermind.TxPool/LightTransaction.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public LightTransaction(Transaction fullTx)
2828
GasBottleneck = fullTx.GasBottleneck;
2929
Timestamp = fullTx.Timestamp;
3030
PoolIndex = fullTx.PoolIndex;
31-
ProofVersion = (fullTx.NetworkWrapper as ShardBlobNetworkWrapper)?.Version ?? default;
31+
ProofVersion = fullTx.GetProofVersion();
3232
_size = fullTx.GetLength();
3333
}
3434

@@ -63,5 +63,7 @@ public LightTransaction(
6363
_size = size;
6464
}
6565

66-
public ProofVersion ProofVersion { get; set; }
66+
public ProofVersion? ProofVersion { get; set; }
67+
68+
public override ProofVersion? GetProofVersion() => ProofVersion;
6769
}

src/Nethermind/Nethermind.TxPool/SpecDrivenTxGossipPolicy.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,5 @@ public class SpecDrivenTxGossipPolicy(IChainHeadInfoProvider chainHeadInfoProvid
1010
private IChainHeadInfoProvider ChainHeadInfoProvider { get; } = chainHeadInfoProvider;
1111

1212
public bool ShouldGossipTransaction(Transaction tx) =>
13-
!tx.SupportsBlobs || (tx.NetworkWrapper as ShardBlobNetworkWrapper)?.Version == ChainHeadInfoProvider.CurrentProofVersion;
13+
!tx.SupportsBlobs || tx.GetProofVersion() == ChainHeadInfoProvider.CurrentProofVersion;
1414
}

src/Nethermind/Nethermind.TxPool/TransactionExtensions.cs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,5 @@ internal static bool IsOverflowInTxCostAndValue(this Transaction tx, out UInt256
9393
=> IsOverflowWhenAddingTxCostToCumulative(tx, UInt256.Zero, out txCost);
9494

9595
public static bool IsInMempoolForm(this Transaction tx) => tx.NetworkWrapper is not null;
96-
97-
public static ProofVersion GetProofVersion(this Transaction mempoolTx) => mempoolTx switch
98-
{
99-
LightTransaction lt => lt.ProofVersion,
100-
{ NetworkWrapper: ShardBlobNetworkWrapper { Version: ProofVersion v } } => v,
101-
_ => ProofVersion.V0,
102-
};
10396
}
10497
}

0 commit comments

Comments
 (0)