Skip to content

Commit

Permalink
Fix after self review (refactor, comments, additional tests).
Browse files Browse the repository at this point in the history
  • Loading branch information
andreibancioiu committed Nov 12, 2024
1 parent 5f8d03f commit 5db19b5
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 5 deletions.
11 changes: 7 additions & 4 deletions factory/state/accountNonceProvider.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,19 @@ func (provider *accountNonceProvider) SetAccountsAdapter(accountsAdapter state.A
return nil
}

// GetAccountNonce returns the nonce for an account
// GetAccountNonce returns the nonce for an account.
// Will be called by "shardedTxPool" on every transaction added to the pool.
func (provider *accountNonceProvider) GetAccountNonce(address []byte) (uint64, error) {
provider.mutex.RLock()
defer provider.mutex.RUnlock()
accountsAdapter := provider.accountsAdapter
provider.mutex.RUnlock()

if check.IfNil(provider.accountsAdapter) {
// No need for double check locking here (we are just guarding against a programming mistake, not against a specific runtime condition).
if check.IfNil(accountsAdapter) {
return 0, errors.ErrNilAccountsAdapter
}

account, err := provider.accountsAdapter.GetExistingAccount(address)
account, err := accountsAdapter.GetExistingAccount(address)
if err != nil {
return 0, err
}
Expand Down
113 changes: 113 additions & 0 deletions factory/state/accountNonceProvider_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package state

import (
"bytes"
"fmt"
"testing"

"github.com/multiversx/mx-chain-go/errors"
"github.com/multiversx/mx-chain-go/testscommon/state"
vmcommon "github.com/multiversx/mx-chain-vm-common-go"
"github.com/stretchr/testify/require"
)

func TestAccountNonceProvider_SetAccountsAdapter(t *testing.T) {
t.Parallel()

t.Run("with a nil the accounts adapter", func(t *testing.T) {
t.Parallel()

provider, err := NewAccountNonceProvider(nil)
require.NoError(t, err)
require.NotNil(t, provider)

err = provider.SetAccountsAdapter(nil)
require.Error(t, errors.ErrNilAccountsAdapter)
})

t.Run("with a non-nil accounts adapter", func(t *testing.T) {
t.Parallel()

provider, err := NewAccountNonceProvider(nil)
require.NoError(t, err)
require.NotNil(t, provider)

err = provider.SetAccountsAdapter(&state.AccountsStub{})
require.NoError(t, err)
})
}

func TestAccountNonceProvider_GetAccountNonce(t *testing.T) {
t.Parallel()

t.Run("without a backing the accounts adapter", func(t *testing.T) {
t.Parallel()

provider, err := NewAccountNonceProvider(nil)
require.NoError(t, err)
require.NotNil(t, provider)

nonce, err := provider.GetAccountNonce(nil)
require.Error(t, errors.ErrNilAccountsAdapter)
require.Equal(t, uint64(0), nonce)
})

t.Run("with a backing accounts adapter (provided in constructor)", func(t *testing.T) {
t.Parallel()

userAddress := []byte("alice")
accounts := &state.AccountsStub{}
accounts.GetExistingAccountCalled = func(address []byte) (vmcommon.AccountHandler, error) {
if bytes.Compare(address, userAddress) != 0 {
return nil, fmt.Errorf("account not found: %s", address)
}

return &state.UserAccountStub{
Nonce: 42,

Check failure on line 66 in factory/state/accountNonceProvider_test.go

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest)

unknown field Nonce in struct literal of type "github.com/multiversx/mx-chain-go/testscommon/state".UserAccountStub

Check failure on line 66 in factory/state/accountNonceProvider_test.go

View workflow job for this annotation

GitHub Actions / golangci linter

unknown field Nonce in struct literal of type "github.com/multiversx/mx-chain-go/testscommon/state".UserAccountStub

Check failure on line 66 in factory/state/accountNonceProvider_test.go

View workflow job for this annotation

GitHub Actions / Build (macos-latest)

unknown field Nonce in struct literal of type "github.com/multiversx/mx-chain-go/testscommon/state".UserAccountStub
}, nil
}

provider, err := NewAccountNonceProvider(accounts)
require.NoError(t, err)
require.NotNil(t, provider)

nonce, err := provider.GetAccountNonce(userAddress)
require.NoError(t, err)
require.Equal(t, uint64(42), nonce)

nonce, err = provider.GetAccountNonce([]byte("bob"))
require.ErrorContains(t, err, "account not found: bob")
require.Equal(t, uint64(0), nonce)
})

t.Run("with a backing accounts adapter (provided using setter)", func(t *testing.T) {
t.Parallel()

userAddress := []byte("alice")
accounts := &state.AccountsStub{}
accounts.GetExistingAccountCalled = func(address []byte) (vmcommon.AccountHandler, error) {
if bytes.Compare(address, userAddress) != 0 {
return nil, fmt.Errorf("account not found: %s", address)
}

return &state.UserAccountStub{
Nonce: 42,

Check failure on line 94 in factory/state/accountNonceProvider_test.go

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest)

unknown field Nonce in struct literal of type "github.com/multiversx/mx-chain-go/testscommon/state".UserAccountStub

Check failure on line 94 in factory/state/accountNonceProvider_test.go

View workflow job for this annotation

GitHub Actions / golangci linter

unknown field Nonce in struct literal of type "github.com/multiversx/mx-chain-go/testscommon/state".UserAccountStub (typecheck)

Check failure on line 94 in factory/state/accountNonceProvider_test.go

View workflow job for this annotation

GitHub Actions / Build (macos-latest)

unknown field Nonce in struct literal of type "github.com/multiversx/mx-chain-go/testscommon/state".UserAccountStub
}, nil
}

provider, err := NewAccountNonceProvider(nil)
require.NoError(t, err)
require.NotNil(t, provider)

err = provider.SetAccountsAdapter(accounts)
require.NoError(t, err)

nonce, err := provider.GetAccountNonce(userAddress)
require.NoError(t, err)
require.Equal(t, uint64(42), nonce)

nonce, err = provider.GetAccountNonce([]byte("bob"))
require.ErrorContains(t, err, "account not found: bob")
require.Equal(t, uint64(0), nonce)
})
}
4 changes: 4 additions & 0 deletions process/block/baseProcess.go
Original file line number Diff line number Diff line change
Expand Up @@ -1434,6 +1434,10 @@ func (bp *baseProcessor) updateStateStorage(
func (bp *baseProcessor) RevertCurrentBlock() {
bp.revertAccountState()
bp.revertScheduledInfo()

// In case of a reverted block, we ask the mempool to forget all the nonces of the accounts,
// so that it doesn't make badly informed decisions (transactions skipping) in the upcoming selections.
// Called synchronously (not in a goroutine): ~5 milliseconds for 100k accounts in the mempool.
bp.txCoordinator.ForgetAllAccountNoncesInMempool()
}

Expand Down
2 changes: 1 addition & 1 deletion process/block/preprocess/miniBlockBuilder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,6 @@ func createWrappedTransaction(
Size: int64(len(txMarshalled)),
}

wrappedTx.PricePerUnit.Store(1000000000)
wrappedTx.PricePerUnit.Store(1_000_000_000)
return wrappedTx
}

0 comments on commit 5db19b5

Please sign in to comment.