Skip to content

Commit

Permalink
Make nonce nullable
Browse files Browse the repository at this point in the history
  • Loading branch information
dimriou committed Nov 27, 2024
1 parent cecb0e1 commit 91e35ea
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 33 deletions.
10 changes: 8 additions & 2 deletions core/chains/evm/txm/attempt_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,11 @@ func (a *attemptBuilder) newLegacyAttempt(ctx context.Context, tx *types.Transac
toAddress = tx.ToAddress
value = tx.Value
}
if tx.Nonce == nil {
return nil, fmt.Errorf("failed to create attempt for txID: %v: nonce empty", tx.ID)
}
legacyTx := evmtypes.LegacyTx{
Nonce: tx.Nonce,
Nonce: *tx.Nonce,
To: &toAddress,
Value: value,
Gas: estimatedGasLimit,
Expand Down Expand Up @@ -126,8 +129,11 @@ func (a *attemptBuilder) newDynamicFeeAttempt(ctx context.Context, tx *types.Tra
toAddress = tx.ToAddress
value = tx.Value
}
if tx.Nonce == nil {
return nil, fmt.Errorf("failed to create attempt for txID: %v: nonce empty", tx.ID)
}
dynamicTx := evmtypes.DynamicFeeTx{
Nonce: tx.Nonce,
Nonce: *tx.Nonce,
To: &toAddress,
Value: value,
Gas: estimatedGasLimit,
Expand Down
26 changes: 16 additions & 10 deletions core/chains/evm/txm/storage/inmemory_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (m *InMemoryStore) CreateEmptyUnconfirmedTransaction(nonce uint64, gasLimit
emptyTx := &types.Transaction{
ID: m.txIDCount,
ChainID: m.chainID,
Nonce: nonce,
Nonce: &nonce,
FromAddress: m.address,
ToAddress: common.Address{},
Value: big.NewInt(0),
Expand Down Expand Up @@ -165,28 +165,34 @@ func (m *InMemoryStore) FetchUnconfirmedTransactionAtNonceWithCount(latestNonce
return
}

func (m *InMemoryStore) MarkTransactionsConfirmed(latestNonce uint64) ([]uint64, []uint64) {
func (m *InMemoryStore) MarkTransactionsConfirmed(latestNonce uint64) ([]uint64, []uint64, error) {
m.Lock()
defer m.Unlock()

var confirmedTransactionIDs []uint64
for _, tx := range m.UnconfirmedTransactions {
if tx.Nonce < latestNonce {
if tx.Nonce == nil {
return nil, nil, fmt.Errorf("nonce for txID: %v is empty", tx.ID)
}
if *tx.Nonce < latestNonce {
tx.State = types.TxConfirmed
confirmedTransactionIDs = append(confirmedTransactionIDs, tx.ID)
m.ConfirmedTransactions[tx.Nonce] = tx
delete(m.UnconfirmedTransactions, tx.Nonce)
m.ConfirmedTransactions[*tx.Nonce] = tx
delete(m.UnconfirmedTransactions, *tx.Nonce)
}
}

var unconfirmedTransactionIDs []uint64
for _, tx := range m.ConfirmedTransactions {
if tx.Nonce >= latestNonce {
if tx.Nonce == nil {
return nil, nil, fmt.Errorf("nonce for txID: %v is empty", tx.ID)
}
if *tx.Nonce >= latestNonce {
tx.State = types.TxUnconfirmed
tx.LastBroadcastAt = time.Time{} // Mark reorged transaction as if it wasn't broadcasted before
unconfirmedTransactionIDs = append(unconfirmedTransactionIDs, tx.ID)
m.UnconfirmedTransactions[tx.Nonce] = tx
delete(m.ConfirmedTransactions, tx.Nonce)
m.UnconfirmedTransactions[*tx.Nonce] = tx
delete(m.ConfirmedTransactions, *tx.Nonce)
}
}

Expand All @@ -197,7 +203,7 @@ func (m *InMemoryStore) MarkTransactionsConfirmed(latestNonce uint64) ([]uint64,
}
sort.Slice(confirmedTransactionIDs, func(i, j int) bool { return confirmedTransactionIDs[i] < confirmedTransactionIDs[j] })
sort.Slice(unconfirmedTransactionIDs, func(i, j int) bool { return unconfirmedTransactionIDs[i] < unconfirmedTransactionIDs[j] })
return confirmedTransactionIDs, unconfirmedTransactionIDs
return confirmedTransactionIDs, unconfirmedTransactionIDs, nil
}

func (m *InMemoryStore) MarkUnconfirmedTransactionPurgeable(nonce uint64) error {
Expand Down Expand Up @@ -249,7 +255,7 @@ func (m *InMemoryStore) UpdateUnstartedTransactionWithNonce(nonce uint64) (*type
}

tx := m.UnstartedTransactions[0]
tx.Nonce = nonce
tx.Nonce = &nonce
tx.State = types.TxUnconfirmed

m.UnstartedTransactions = m.UnstartedTransactions[1:]
Expand Down
2 changes: 1 addition & 1 deletion core/chains/evm/txm/storage/inmemory_store_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (m *InMemoryStoreManager) FetchUnconfirmedTransactionAtNonceWithCount(_ con

func (m *InMemoryStoreManager) MarkTransactionsConfirmed(_ context.Context, nonce uint64, fromAddress common.Address) (confirmedTxIDs []uint64, unconfirmedTxIDs []uint64, err error) {
if store, exists := m.InMemoryStoreMap[fromAddress]; exists {
confirmedTxIDs, unconfirmedTxIDs = store.MarkTransactionsConfirmed(nonce)
confirmedTxIDs, unconfirmedTxIDs, err = store.MarkTransactionsConfirmed(nonce)
return
}
return nil, nil, fmt.Errorf(StoreNotFoundForAddress, fromAddress)
Expand Down
29 changes: 17 additions & 12 deletions core/chains/evm/txm/storage/inmemory_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func TestFetchUnconfirmedTransactionAtNonceWithCount(t *testing.T) {
_, err := insertUnconfirmedTransaction(m, nonce)
require.NoError(t, err)
tx, count = m.FetchUnconfirmedTransactionAtNonceWithCount(0)
assert.Equal(t, tx.Nonce, nonce)
assert.Equal(t, *tx.Nonce, nonce)
assert.Equal(t, 1, count)
}

Expand All @@ -192,7 +192,8 @@ func TestMarkTransactionsConfirmed(t *testing.T) {

t.Run("returns 0 if there are no transactions", func(t *testing.T) {
m := NewInMemoryStore(logger.Test(t), fromAddress, testutils.FixtureChainID)
un, cn := m.MarkTransactionsConfirmed(100)
un, cn, err := m.MarkTransactionsConfirmed(100)
require.NoError(t, err)
assert.Empty(t, un)
assert.Empty(t, cn)
})
Expand All @@ -205,7 +206,7 @@ func TestMarkTransactionsConfirmed(t *testing.T) {
ctx2, err := insertUnconfirmedTransaction(m, 1)
require.NoError(t, err)

ctxs, utxs := m.MarkTransactionsConfirmed(1)
ctxs, utxs, err := m.MarkTransactionsConfirmed(1)
require.NoError(t, err)
assert.Equal(t, types.TxConfirmed, ctx1.State)
assert.Equal(t, types.TxUnconfirmed, ctx2.State)
Expand All @@ -221,7 +222,7 @@ func TestMarkTransactionsConfirmed(t *testing.T) {
ctx2, err := insertConfirmedTransaction(m, 1)
require.NoError(t, err)

ctxs, utxs := m.MarkTransactionsConfirmed(1)
ctxs, utxs, err := m.MarkTransactionsConfirmed(1)
require.NoError(t, err)
assert.Equal(t, types.TxConfirmed, ctx1.State)
assert.Equal(t, types.TxUnconfirmed, ctx2.State)
Expand All @@ -236,7 +237,8 @@ func TestMarkTransactionsConfirmed(t *testing.T) {
require.NoError(t, err)
}
assert.Len(t, m.ConfirmedTransactions, maxQueuedTransactions)
m.MarkTransactionsConfirmed(maxQueuedTransactions)
_, _, err := m.MarkTransactionsConfirmed(maxQueuedTransactions)
require.NoError(t, err)
assert.Len(t, m.ConfirmedTransactions, (maxQueuedTransactions - maxQueuedTransactions/pruneSubset))
})
}
Expand Down Expand Up @@ -324,7 +326,7 @@ func TestUpdateUnstartedTransactionWithNonce(t *testing.T) {

tx, err := m.UpdateUnstartedTransactionWithNonce(nonce)
require.NoError(t, err)
assert.Equal(t, nonce, tx.Nonce)
assert.Equal(t, nonce, *tx.Nonce)
assert.Equal(t, types.TxUnconfirmed, tx.State)
})
}
Expand All @@ -335,9 +337,10 @@ func TestDeleteAttemptForUnconfirmedTx(t *testing.T) {
fromAddress := testutils.NewAddress()
t.Run("fails if corresponding unconfirmed transaction for attempt was not found", func(t *testing.T) {
m := NewInMemoryStore(logger.Test(t), fromAddress, testutils.FixtureChainID)
tx := &types.Transaction{Nonce: 0}
var nonce uint64
tx := &types.Transaction{Nonce: &nonce}
attempt := &types.Attempt{TxID: 0}
err := m.DeleteAttemptForUnconfirmedTx(tx.Nonce, attempt)
err := m.DeleteAttemptForUnconfirmedTx(*tx.Nonce, attempt)
require.Error(t, err)
})

Expand Down Expand Up @@ -388,11 +391,12 @@ func insertUnstartedTransaction(m *InMemoryStore) *types.Transaction {
m.Lock()
defer m.Unlock()

var nonce uint64
m.txIDCount++
tx := &types.Transaction{
ID: m.txIDCount,
ChainID: testutils.FixtureChainID,
Nonce: 0,
Nonce: &nonce,
FromAddress: m.address,
ToAddress: testutils.NewAddress(),
Value: big.NewInt(0),
Expand All @@ -413,7 +417,7 @@ func insertUnconfirmedTransaction(m *InMemoryStore, nonce uint64) (*types.Transa
tx := &types.Transaction{
ID: m.txIDCount,
ChainID: testutils.FixtureChainID,
Nonce: nonce,
Nonce: &nonce,
FromAddress: m.address,
ToAddress: testutils.NewAddress(),
Value: big.NewInt(0),
Expand All @@ -438,7 +442,7 @@ func insertConfirmedTransaction(m *InMemoryStore, nonce uint64) (*types.Transact
tx := &types.Transaction{
ID: m.txIDCount,
ChainID: testutils.FixtureChainID,
Nonce: nonce,
Nonce: &nonce,
FromAddress: m.address,
ToAddress: testutils.NewAddress(),
Value: big.NewInt(0),
Expand All @@ -459,11 +463,12 @@ func insertFataTransaction(m *InMemoryStore) *types.Transaction {
m.Lock()
defer m.Unlock()

var nonce uint64
m.txIDCount++
tx := &types.Transaction{
ID: m.txIDCount,
ChainID: testutils.FixtureChainID,
Nonce: 0,
Nonce: &nonce,
FromAddress: m.address,
ToAddress: testutils.NewAddress(),
Value: big.NewInt(0),
Expand Down
21 changes: 14 additions & 7 deletions core/chains/evm/txm/txm.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,9 @@ func (t *Txm) broadcastTransaction(ctx context.Context, address common.Address)
if tx == nil {
return false, nil
}
tx.Nonce = t.getNonce(address)
t.setNonce(address, tx.Nonce+1)
nonce := t.getNonce(address)
tx.Nonce = &nonce
t.setNonce(address, *tx.Nonce+1)
tx.State = types.TxUnconfirmed

if err := t.createAndSendAttempt(ctx, tx, address); err != nil {
Expand All @@ -302,14 +303,20 @@ func (t *Txm) createAndSendAttempt(ctx context.Context, tx *types.Transaction, a
return err
}

if err = t.txStore.AppendAttemptToTransaction(ctx, tx.Nonce, address, attempt); err != nil {
if tx.Nonce == nil {
return fmt.Errorf("nonce for txID: %v is empty", tx.ID)
}
if err = t.txStore.AppendAttemptToTransaction(ctx, *tx.Nonce, address, attempt); err != nil {
return err
}

return t.sendTransactionWithError(ctx, tx, attempt, address)
}

func (t *Txm) sendTransactionWithError(ctx context.Context, tx *types.Transaction, attempt *types.Attempt, address common.Address) (err error) {
if tx.Nonce == nil {
return fmt.Errorf("nonce for txID: %v is empty", tx.ID)
}
start := time.Now()
txErr := t.client.SendTransaction(ctx, tx, attempt)
tx.AttemptCount++
Expand All @@ -323,13 +330,13 @@ func (t *Txm) sendTransactionWithError(ctx context.Context, tx *types.Transactio
if err != nil {
return err
}
if pendingNonce <= tx.Nonce {
if pendingNonce <= *tx.Nonce {
t.lggr.Debugf("Pending nonce for txID: %v didn't increase. PendingNonce: %d, TxNonce: %d", tx.ID, pendingNonce, tx.Nonce)
return nil
}
}

return t.txStore.UpdateTransactionBroadcast(ctx, attempt.TxID, tx.Nonce, attempt.Hash, address)
return t.txStore.UpdateTransactionBroadcast(ctx, attempt.TxID, *tx.Nonce, attempt.Hash, address)
}

func (t *Txm) backfillTransactions(ctx context.Context, address common.Address) (bool, error) {
Expand All @@ -355,7 +362,7 @@ func (t *Txm) backfillTransactions(ctx context.Context, address common.Address)
return false, err // TODO: add backoff to optimize requests
}

if tx == nil || tx.Nonce != latestNonce {
if tx == nil || *tx.Nonce != latestNonce {
t.lggr.Warnf("Nonce gap at nonce: %d - address: %v. Creating a new transaction\n", latestNonce, address)
return false, t.createAndSendEmptyTx(ctx, latestNonce, address)
} else { //nolint:revive //linter nonsense
Expand All @@ -366,7 +373,7 @@ func (t *Txm) backfillTransactions(ctx context.Context, address common.Address)
}
if isStuck {
tx.IsPurgeable = true
err = t.txStore.MarkUnconfirmedTransactionPurgeable(ctx, tx.Nonce, address)
err = t.txStore.MarkUnconfirmedTransactionPurgeable(ctx, *tx.Nonce, address)
if err != nil {
return false, err
}
Expand Down
2 changes: 1 addition & 1 deletion core/chains/evm/txm/types/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type Transaction struct {
ID uint64
IdempotencyKey *string
ChainID *big.Int
Nonce uint64
Nonce *uint64
FromAddress common.Address
ToAddress common.Address
Value *big.Int
Expand Down

0 comments on commit 91e35ea

Please sign in to comment.