-
Notifications
You must be signed in to change notification settings - Fork 432
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.
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
Fix/6790 return accepted for rejected tx added to broadcast #6889
Changes from 1 commit
ca922d9
1e4670a
401e318
13a767a
919bd2d
14e8b79
7936a1e
ca67a97
6d0a22c
7dfbdb6
4757f6f
018afc9
be126f5
b64dd50
3c19462
926c381
f8546dd
c695cc5
8360abc
031d15e
f97394e
a6150f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ | |
using Nethermind.Crypto; | ||
using Nethermind.Db; | ||
using Nethermind.Evm; | ||
using Nethermind.Evm.Tracing.GethStyle.JavaScript; | ||
Check failure on line 24 in src/Nethermind/Nethermind.TxPool.Test/TxPoolTests.cs GitHub Actions / Build (release, Nethermind)
Check failure on line 24 in src/Nethermind/Nethermind.TxPool.Test/TxPoolTests.cs GitHub Actions / Build (release, Nethermind)
Check failure on line 24 in src/Nethermind/Nethermind.TxPool.Test/TxPoolTests.cs GitHub Actions / Build (debug, Nethermind)
Check failure on line 24 in src/Nethermind/Nethermind.TxPool.Test/TxPoolTests.cs GitHub Actions / Build (debug, Nethermind)
|
||
using Nethermind.Int256; | ||
using Nethermind.Logging; | ||
using Nethermind.Specs; | ||
|
@@ -1638,11 +1639,14 @@ | |
result.Should().Be(expectedResult ? AcceptTxResult.Accepted : AcceptTxResult.FeeTooLowToCompete); | ||
} | ||
|
||
[TestCase] | ||
public void Should_only_add_legacy_and_1559_local_transactions_to_local_pool_when_underpaid() | ||
[TestCase(TxType.Legacy, true)] | ||
[TestCase(TxType.EIP1559, true)] | ||
[TestCase(TxType.Blob, false)] | ||
public void Should_correctly_add_tx_to_local_pool_when_underpaid(TxType txType, bool expectedResult) | ||
{ | ||
ISpecProvider specProvider = GetLondonSpecProvider(); | ||
TxPoolConfig txPoolConfig = new TxPoolConfig { Size = 30 }; | ||
// Should only add legacy and 1559 local transactions to local pool when underpaid | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. applied |
||
ISpecProvider specProvider = GetCancunSpecProvider(); | ||
TxPoolConfig txPoolConfig = new TxPoolConfig { Size = 30, PersistentBlobStorageSize = 0 }; | ||
_txPool = CreatePool(txPoolConfig, specProvider); | ||
|
||
Transaction[] transactions = GetTransactions(GetPeers(3), true, false); | ||
|
@@ -1652,6 +1656,7 @@ | |
EnsureSenderBalance(address, UInt256.MaxValue); | ||
} | ||
|
||
// setup full tx pool | ||
foreach (Transaction transaction in transactions) | ||
{ | ||
transaction.GasPrice = 10.GWei(); | ||
|
@@ -1660,52 +1665,22 @@ | |
|
||
_txPool.GetPendingTransactionsCount().Should().Be(30); | ||
|
||
|
||
// send legacy tx (gasPrice 9 gwei) | ||
Transaction firstTx = Build.A.Transaction | ||
Transaction testTx = Build.A.Transaction | ||
.WithNonce(0) | ||
.WithValue(0) | ||
.WithType(TxType.Legacy) | ||
.WithGasPrice(9.GWei()) | ||
.WithTo(TestItem.AddressD) | ||
.WithType(txType) | ||
.WithShardBlobTxTypeAndFieldsIfBlobTx() | ||
.WithMaxFeePerGas(9.GWei()) | ||
.WithMaxPriorityFeePerGas(9.GWei()) | ||
.WithTo(TestItem.AddressB) | ||
.SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyA).TestObject; | ||
|
||
EnsureSenderBalance(TestItem.PrivateKeyA.Address, UInt256.MaxValue); | ||
_txPool.SubmitTx(firstTx, TxHandlingOptions.PersistentBroadcast).Should().Be(AcceptTxResult.Accepted); | ||
_txPool.GetOwnPendingTransactions().Length.Should().Be(1); | ||
_txPool.GetPendingTransactions().Should().NotContain(firstTx); | ||
|
||
|
||
|
||
// send eip1559 tx (gasPrice 9 gwei) | ||
Transaction secondTx = Build.A.Transaction | ||
.WithNonce(0) | ||
.WithValue(0) | ||
.WithType(TxType.EIP1559) | ||
.WithTo(TestItem.AddressD) | ||
.WithMaxFeePerGas(9.GWei()) | ||
.SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyB).TestObject; | ||
|
||
EnsureSenderBalance(TestItem.PrivateKeyB.Address, UInt256.MaxValue); | ||
_txPool.SubmitTx(secondTx, TxHandlingOptions.PersistentBroadcast).Should().Be(AcceptTxResult.Accepted); | ||
_txPool.GetOwnPendingTransactions().Length.Should().Be(2); | ||
_txPool.GetPendingTransactions().Should().NotContain(secondTx); | ||
|
||
|
||
// send blop tx (gasPrice 9 gwei) | ||
Transaction thirdTx = Build.A.Transaction | ||
.WithNonce(0) | ||
.WithValue(0) | ||
.WithType(TxType.Blob) | ||
.WithTo(TestItem.AddressD) | ||
.WithBlobVersionedHashes(new byte[1][]) | ||
.WithMaxFeePerBlobGas(9.GWei()) | ||
.SignedAndResolved(_ethereumEcdsa, TestItem.PrivateKeyC).TestObject; | ||
|
||
EnsureSenderBalance(TestItem.PrivateKeyC.Address, UInt256.MaxValue); | ||
_txPool.SubmitTx(thirdTx, TxHandlingOptions.PersistentBroadcast).Should().Be(AcceptTxResult.FeeTooLow); | ||
_txPool.GetOwnPendingTransactions().Length.Should().Be(2); | ||
_txPool.GetPendingTransactions().Should().NotContain(thirdTx); | ||
AcceptTxResult result = _txPool.SubmitTx(testTx, TxHandlingOptions.PersistentBroadcast); | ||
result.Should().Be(expectedResult ? AcceptTxResult.Accepted : AcceptTxResult.FeeTooLowToCompete); | ||
_txPool.GetOwnPendingTransactions().Length.Should().Be(expectedResult ? 1 : 0); | ||
_txPool.GetPendingBlobTransactionsCount().Should().Be(0); | ||
_txPool.GetPendingTransactions().Should().NotContain(testTx); | ||
} | ||
|
||
private IDictionary<ITxPoolPeer, PrivateKey> GetPeers(int limit = 100) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and
[TestCase(TxType.AccessList, true)]
to have all typesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe better to have TestCaseSource and generate test cases for all enum items, by default with
true
and just have exception for blobs. In that case if we add new tx type we will by default have a test that expectstrue
here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed the test accordingly to check all the values of
TxType
.