-
Notifications
You must be signed in to change notification settings - Fork 3
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
add market gas price to simulate duplicate nonce behavior #7
Changes from 3 commits
62079ae
16470f5
8cc712d
3647e5e
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 |
---|---|---|
|
@@ -27,6 +27,9 @@ import ( | |
"testing" | ||
"time" | ||
|
||
"github.com/stretchr/testify/assert" | ||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/ethereum/go-ethereum" | ||
"github.com/ethereum/go-ethereum/accounts/abi" | ||
"github.com/ethereum/go-ethereum/accounts/abi/bind" | ||
|
@@ -1336,3 +1339,129 @@ func TestForkResendTx(t *testing.T) { | |
t.Errorf("TX included in wrong block: %d", h) | ||
} | ||
} | ||
|
||
// TestLowGasTxNotBeingMined checks that the lower gas fee tx with same nonce does not get mined | ||
// Steps: | ||
// 1. Send a tx lower than the set market gas price | ||
// 2. Commit to chain, check nonce stays the same | ||
// 3. Send a tx meets the market gas price with same nonce. | ||
// 4. Commit to chain | ||
// 5. Check tx2 is not in pending state and nonce has increased | ||
func TestLowGasTxNotBeingMined(t *testing.T) { | ||
testAddr := crypto.PubkeyToAddress(testKey.PublicKey) | ||
var gas uint64 = 21000 | ||
|
||
sim := simTestBackend(testAddr) | ||
defer sim.Close() | ||
|
||
bgCtx := context.Background() | ||
|
||
// Create tx and send | ||
head, _ := sim.HeaderByNumber(context.Background(), nil) // Should be child's, good enough | ||
gasPrice := new(big.Int).Add(head.BaseFee, big.NewInt(1)) | ||
sim.SetMarketGasPrice(int64(gasPrice.Uint64())) | ||
|
||
lowGasFeeTx := types.NewTx(&types.LegacyTx{ | ||
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. nit: let's try both legacy and dynamic fee txs in this test 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. added both legacy and dynamic fee txs in both test cases! |
||
Nonce: uint64(0), | ||
To: &testAddr, | ||
Value: big.NewInt(1), | ||
GasPrice: new(big.Int).Sub(gasPrice, big.NewInt(1)), | ||
Gas: gas, | ||
}) | ||
signedTx, err := types.SignTx(lowGasFeeTx, types.HomesteadSigner{}, testKey) | ||
require.NoError(t, err) | ||
|
||
// send tx to simulated backend | ||
err = sim.SendTransaction(bgCtx, signedTx) | ||
require.NoError(t, err) | ||
sim.Commit() | ||
|
||
// expect nonce be the same because low gas fee tx will not be mined | ||
nonce, err := sim.PendingNonceAt(bgCtx, testAddr) | ||
require.NoError(t, err) | ||
assert.Equal(t, uint64(0), nonce) | ||
|
||
// send tx with higher gas fee | ||
sufficientGasFeeTx := types.NewTx(&types.LegacyTx{ | ||
Nonce: uint64(0), | ||
To: &testAddr, | ||
Value: big.NewInt(1), | ||
GasPrice: gasPrice, | ||
Gas: gas, | ||
}) | ||
|
||
signedSufficientTx, err := types.SignTx(sufficientGasFeeTx, types.HomesteadSigner{}, testKey) | ||
require.NoError(t, err) | ||
|
||
err = sim.SendTransaction(bgCtx, signedSufficientTx) | ||
require.NoError(t, err) | ||
sim.Commit() | ||
|
||
// the higher gas transaction should have been mined | ||
_, isPending, err := sim.TransactionByHash(bgCtx, signedSufficientTx.Hash()) | ||
require.NoError(t, err) | ||
assert.False(t, isPending) | ||
|
||
// expect nonce has increased | ||
nonce, err = sim.PendingNonceAt(bgCtx, testAddr) | ||
require.NoError(t, err) | ||
assert.Equal(t, uint64(1), nonce) | ||
} | ||
|
||
// TestLowGasTxGetMinedOnceGasFeeDropped checks that lower gas fee txes still stay in mem pool | ||
// and get mined once gas fee requirement has been met | ||
// Steps: | ||
// 1. Send a tx lower than the set market gas price | ||
// 2. Commit to chain, tx does not get mined | ||
// 3. Gas fee drops, commit again | ||
// 4. Check tx get mined | ||
func TestLowGasTxGetMinedOnceGasFeeDropped(t *testing.T) { | ||
testAddr := crypto.PubkeyToAddress(testKey.PublicKey) | ||
var gas uint64 = 21000 | ||
|
||
sim := simTestBackend(testAddr) | ||
defer sim.Close() | ||
|
||
bgCtx := context.Background() | ||
|
||
// Create tx and send | ||
head, _ := sim.HeaderByNumber(context.Background(), nil) // Should be child's, good enough | ||
normalGasPrice := new(big.Int).Add(head.BaseFee, big.NewInt(1)) | ||
highGasPrice := new(big.Int).Add(head.BaseFee, big.NewInt(5)) | ||
|
||
sim.SetMarketGasPrice(int64(highGasPrice.Uint64())) | ||
|
||
normalGasFeeTx := types.NewTx(&types.LegacyTx{ | ||
Nonce: uint64(0), | ||
To: &testAddr, | ||
Value: big.NewInt(1), | ||
GasPrice: normalGasPrice, | ||
Gas: gas, | ||
}) | ||
signedTx, err := types.SignTx(normalGasFeeTx, types.HomesteadSigner{}, testKey) | ||
require.NoError(t, err) | ||
|
||
// send tx to simulated backend | ||
err = sim.SendTransaction(bgCtx, signedTx) | ||
require.NoError(t, err) | ||
sim.Commit() | ||
|
||
// check that the nonce stays the same because nothing has been mined | ||
pendingNonce, err := sim.PendingNonceAt(bgCtx, testAddr) | ||
require.NoError(t, err) | ||
assert.Equal(t, uint64(0), pendingNonce) | ||
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. let's do another commit and nonce check after this one? so we go through the cycle of passing forward the stuckTransactions twice. 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. this test set up is the following hmmm, are you suggesting after (3) we should commit again even though we dont have new tx and nothing in stuck transactions? |
||
|
||
// gas fee has dropped | ||
sim.SetMarketGasPrice(int64(normalGasPrice.Uint64())) | ||
sim.Commit() | ||
|
||
// nonce has increased | ||
pendingNonce, err = sim.PendingNonceAt(bgCtx, testAddr) | ||
require.NoError(t, err) | ||
assert.Equal(t, uint64(1), pendingNonce) | ||
|
||
// the transaction should have been mined | ||
_, isPending, err := sim.TransactionByHash(bgCtx, signedTx.Hash()) | ||
require.NoError(t, err) | ||
assert.False(t, isPending) | ||
} |
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.
can we instead just put them back on the pending block?
similarly to how SendTransaction recreates the pendingBlock
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.
we cannot put them to the pending block, the txes in the pending block determine whats the next nonce available. if they are stuck, they should not be in pending block. because that means we cannot send the transaction with the same nonce. it will be invalid/nonce too low.