From ac0701a0070537285fc54491d2db94e2926d232a Mon Sep 17 00:00:00 2001 From: Unique-Divine Date: Sat, 27 Apr 2024 07:26:57 -0500 Subject: [PATCH] test(eth-rpc): more tests for types dir --- eth/rpc/types/addrlock.go | 39 ---------- eth/rpc/types/addrlocker.go | 48 ++++++++++++ eth/rpc/types/addrlocker_test.go | 126 +++++++++++++++++++++++++++++++ eth/rpc/types/block.go | 4 +- eth/rpc/types/block_test.go | 81 +++++++++++++++----- eth/rpc/types/query_client.go | 5 ++ eth/rpc/types/rpc.go | 5 +- 7 files changed, 245 insertions(+), 63 deletions(-) delete mode 100644 eth/rpc/types/addrlock.go create mode 100644 eth/rpc/types/addrlocker.go create mode 100644 eth/rpc/types/addrlocker_test.go diff --git a/eth/rpc/types/addrlock.go b/eth/rpc/types/addrlock.go deleted file mode 100644 index 2061d1206..000000000 --- a/eth/rpc/types/addrlock.go +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright (c) 2023-2024 Nibi, Inc. -package types - -import ( - "sync" - - "github.com/ethereum/go-ethereum/common" -) - -// AddrLocker is a mutex structure used to avoid querying outdated account data -type AddrLocker struct { - mu sync.Mutex - locks map[common.Address]*sync.Mutex -} - -// lock returns the lock of the given address. -func (l *AddrLocker) lock(address common.Address) *sync.Mutex { - l.mu.Lock() - defer l.mu.Unlock() - if l.locks == nil { - l.locks = make(map[common.Address]*sync.Mutex) - } - if _, ok := l.locks[address]; !ok { - l.locks[address] = new(sync.Mutex) - } - return l.locks[address] -} - -// LockAddr locks an account's mutex. This is used to prevent another tx getting the -// same nonce until the lock is released. The mutex prevents the (an identical nonce) from -// being read again during the time that the first transaction is being signed. -func (l *AddrLocker) LockAddr(address common.Address) { - l.lock(address).Lock() -} - -// UnlockAddr unlocks the mutex of the given account. -func (l *AddrLocker) UnlockAddr(address common.Address) { - l.lock(address).Unlock() -} diff --git a/eth/rpc/types/addrlocker.go b/eth/rpc/types/addrlocker.go new file mode 100644 index 000000000..a7aca52a6 --- /dev/null +++ b/eth/rpc/types/addrlocker.go @@ -0,0 +1,48 @@ +// Copyright (c) 2023-2024 Nibi, Inc. +package types + +import ( + "sync" + + "github.com/ethereum/go-ethereum/common" +) + +// AddrLocker is a mutex (mutual exclusion lock) structure used to avoid querying +// outdated account data. It prevents data races by allowing only one goroutine +// to access critical sections at a time. +type AddrLocker struct { + // mu protects access to the locks map + mu sync.Mutex + locks map[common.Address]*sync.Mutex +} + +// lock returns the mutex lock of the given Ethereum address. If no mutex exists +// for the address, it creates a new one. This function ensures that each address +// has exactly one mutex associated with it, and it is thread-safe. +// +// The returned mutex is not locked; callers are responsible for locking and +// unlocking it as necessary. +func (l *AddrLocker) lock(address common.Address) *sync.Mutex { + l.mu.Lock() + defer l.mu.Unlock() + if l.locks == nil { + l.locks = make(map[common.Address]*sync.Mutex) + } + if _, ok := l.locks[address]; !ok { + l.locks[address] = new(sync.Mutex) + } + return l.locks[address] +} + +// LockAddr acquires the mutex for a specific address, blocking if it is already +// held by another goroutine. The mutex lock prevents an identical nonce from +// being read again during the time that the first transaction is being signed. +func (l *AddrLocker) LockAddr(address common.Address) { + l.lock(address).Lock() +} + +// UnlockAddr unlocks the mutex for a specific address, allowing other goroutines +// to acquire it. +func (l *AddrLocker) UnlockAddr(address common.Address) { + l.lock(address).Unlock() +} diff --git a/eth/rpc/types/addrlocker_test.go b/eth/rpc/types/addrlocker_test.go new file mode 100644 index 000000000..69f00c961 --- /dev/null +++ b/eth/rpc/types/addrlocker_test.go @@ -0,0 +1,126 @@ +package types_test + +import ( + "sync" + "testing" + "time" + + "github.com/ethereum/go-ethereum/common" + "github.com/stretchr/testify/suite" + + rpc "github.com/NibiruChain/nibiru/eth/rpc/types" +) + +type SuiteAddrLocker struct { + suite.Suite +} + +func TestSuiteAddrLocker(t *testing.T) { + suite.Run(t, new(SuiteAddrLocker)) +} + +// TestLockAddr: This test checks that the lock mechanism prevents multiple +// goroutines from entering critical sections of code simultaneously for the same +// address. +func (s *SuiteAddrLocker) TestLockAddr() { + // Setup: Lock the address + locker := &rpc.AddrLocker{} + addr := common.HexToAddress("0x123") + locker.LockAddr(addr) + + // Concurrent Lock Attempt: Attempt to lock again in a separate goroutine. If + // the initial lock is effective, this attempt should block and not complete + // immediately. + done := make(chan bool) + go func() { + locker.LockAddr(addr) // This should block if the first lock is effective + done <- true + }() + + // Assertion: A select statement is used to check if the channel receives a + // value, which would indicate the lock did not block as expected. + select { + case <-done: + s.Fail("LockAddr did not block the second call as expected") + default: + // expected behavior, continue test + } + + // Cleanup: Unlock and allow the goroutine to proceed + locker.UnlockAddr(addr) + <-done // Ensure goroutine completes +} + +func (s *SuiteAddrLocker) TestUnlockAddr() { + // Setup: Lock the address + locker := &rpc.AddrLocker{} + addr := common.HexToAddress("0x123") + locker.LockAddr(addr) + + locker.UnlockAddr(addr) + + // Try re-locking to test if unlock was successful + locked := make(chan bool) + go func() { + locker.LockAddr(addr) // This should not block if unlock worked + locked <- true + locker.UnlockAddr(addr) + }() + + select { + case <-locked: + // expected behavior, continue + case <-time.After(time.Second): + s.Fail("UnlockAddr did not effectively unlock the mutex") + } +} + +func (s *SuiteAddrLocker) TestMultipleAddresses() { + locker := &rpc.AddrLocker{} + addr1 := common.HexToAddress("0x123") + addr2 := common.HexToAddress("0x456") + + locker.LockAddr(addr1) + locked := make(chan bool) + + go func() { + locker.LockAddr(addr2) // This should not block if locks are address-specific + locked <- true + locker.UnlockAddr(addr2) + }() + + select { + case <-locked: + // expected behavior, continue + case <-time.After(time.Second): + s.Fail("Locks are not address-specific as expected") + } + + locker.UnlockAddr(addr1) +} + +// TestConcurrentAccess: Tests the system's behavior under high concurrency, +// specifically ensuring that the lock can handle multiple locking and unlocking +// operations on the same address without leading to race conditions or +// deadlocks. +func (s *SuiteAddrLocker) TestConcurrentAccess() { + locker := &rpc.AddrLocker{} + addr := common.HexToAddress("0x789") + var wg sync.WaitGroup + + // Spawn 100 goroutines, each locking and unlocking the same address. + // Each routine will hod the lock briefly to simulate work done during the + // lock (like an Ethereum query). + for i := 0; i < 100; i++ { + wg.Add(1) + go func() { + locker.LockAddr(addr) + time.Sleep(time.Millisecond * 5) // Simulate work + locker.UnlockAddr(addr) + wg.Done() + }() + } + + // Cleanup: Wait for all goroutines to complete + wg.Wait() +} diff --git a/eth/rpc/types/block.go b/eth/rpc/types/block.go index 783587427..9669782ea 100644 --- a/eth/rpc/types/block.go +++ b/eth/rpc/types/block.go @@ -48,12 +48,12 @@ func NewBlockNumber(n *big.Int) BlockNumber { return BlockNumber(n.Int64()) } -// ContextWithHeight wraps a context with the a gRPC block height header. If the +// NewContextWithHeight wraps a context with the a gRPC block height header. If the // provided height is 0, it will return an empty context and the gRPC query will // use the latest block height for querying. Note that all metadata gets processed // and removed by tendermint layer, so it wont be accessible at gRPC server // level. -func ContextWithHeight(height int64) context.Context { +func NewContextWithHeight(height int64) context.Context { if height == 0 { return context.Background() } diff --git a/eth/rpc/types/block_test.go b/eth/rpc/types/block_test.go index 1cbb11f61..08c1da28a 100644 --- a/eth/rpc/types/block_test.go +++ b/eth/rpc/types/block_test.go @@ -2,13 +2,54 @@ package types import ( "fmt" + "math/big" "testing" + grpctypes "github.com/cosmos/cosmos-sdk/types/grpc" "github.com/ethereum/go-ethereum/common" - "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + "google.golang.org/grpc/metadata" ) -func TestUnmarshalBlockNumberOrHash(t *testing.T) { +type BlockSuite struct { + suite.Suite +} + +func TestBlockSuite(t *testing.T) { + suite.Run(t, new(BlockSuite)) +} + +func (s *BlockSuite) TestNewBlockNumber() { + bigInt := big.NewInt(1) + bn := NewBlockNumber(bigInt) + bnInt64 := bn.Int64() + bnTmHeight := bn.TmHeight() + s.EqualValues(bnInt64, *bnTmHeight) + s.EqualValues(bigInt.Int64(), bnInt64) +} + +func (s *BlockSuite) TestNewContextWithHeight() { + // Test with zero height + ctxZero := NewContextWithHeight(0) + _, ok := metadata.FromOutgoingContext(ctxZero) + s.False(ok, "No metadata should be present for height 0") + + // Test with non-zero height + height := int64(10) + ctxTen := NewContextWithHeight(height) + md, ok := metadata.FromOutgoingContext(ctxTen) + s.True(ok, "Metadata should be present for non-zero height") + s.NotEmpty(md, "Metadata should not be empty") + + heightStr, ok := md[grpctypes.GRPCBlockHeightHeader] + s.True(ok, grpctypes.GRPCBlockHeightHeader, " metadata should be present") + s.Require().Len(heightStr, 1, + fmt.Sprintf("There should be exactly one %s value", grpctypes.GRPCBlockHeightHeader)) + s.Equal(fmt.Sprintf("%d", height), heightStr[0], + "The height value in metadata should match the provided height") +} + +func (s *BlockSuite) TestUnmarshalBlockNumberOrHash() { bnh := new(BlockNumberOrHash) testCases := []struct { @@ -18,20 +59,20 @@ func TestUnmarshalBlockNumberOrHash(t *testing.T) { expPass bool }{ { - "JSON input with block hash", - []byte("{\"blockHash\": \"0x579917054e325746fda5c3ee431d73d26255bc4e10b51163862368629ae19739\"}"), - func() { - require.Equal(t, *bnh.BlockHash, common.HexToHash("0x579917054e325746fda5c3ee431d73d26255bc4e10b51163862368629ae19739")) - require.Nil(t, bnh.BlockNumber) + msg: "JSON input with block hash", + input: []byte("{\"blockHash\": \"0x579917054e325746fda5c3ee431d73d26255bc4e10b51163862368629ae19739\"}"), + malleate: func() { + s.Equal(*bnh.BlockHash, common.HexToHash("0x579917054e325746fda5c3ee431d73d26255bc4e10b51163862368629ae19739")) + s.Nil(bnh.BlockNumber) }, - true, + expPass: true, }, { "JSON input with block number", []byte("{\"blockNumber\": \"0x35\"}"), func() { - require.Equal(t, *bnh.BlockNumber, BlockNumber(0x35)) - require.Nil(t, bnh.BlockHash) + s.Equal(*bnh.BlockNumber, BlockNumber(0x35)) + s.Nil(bnh.BlockHash) }, true, }, @@ -39,8 +80,8 @@ func TestUnmarshalBlockNumberOrHash(t *testing.T) { "JSON input with block number latest", []byte("{\"blockNumber\": \"latest\"}"), func() { - require.Equal(t, *bnh.BlockNumber, EthLatestBlockNumber) - require.Nil(t, bnh.BlockHash) + s.Equal(*bnh.BlockNumber, EthLatestBlockNumber) + s.Nil(bnh.BlockHash) }, true, }, @@ -55,8 +96,8 @@ func TestUnmarshalBlockNumberOrHash(t *testing.T) { "String input with block hash", []byte("\"0x579917054e325746fda5c3ee431d73d26255bc4e10b51163862368629ae19739\""), func() { - require.Equal(t, *bnh.BlockHash, common.HexToHash("0x579917054e325746fda5c3ee431d73d26255bc4e10b51163862368629ae19739")) - require.Nil(t, bnh.BlockNumber) + s.Equal(*bnh.BlockHash, common.HexToHash("0x579917054e325746fda5c3ee431d73d26255bc4e10b51163862368629ae19739")) + s.Nil(bnh.BlockNumber) }, true, }, @@ -64,8 +105,8 @@ func TestUnmarshalBlockNumberOrHash(t *testing.T) { "String input with block number", []byte("\"0x35\""), func() { - require.Equal(t, *bnh.BlockNumber, BlockNumber(0x35)) - require.Nil(t, bnh.BlockHash) + s.Equal(*bnh.BlockNumber, BlockNumber(0x35)) + s.Nil(bnh.BlockHash) }, true, }, @@ -73,8 +114,8 @@ func TestUnmarshalBlockNumberOrHash(t *testing.T) { "String input with block number latest", []byte("\"latest\""), func() { - require.Equal(t, *bnh.BlockNumber, EthLatestBlockNumber) - require.Nil(t, bnh.BlockHash) + s.Equal(*bnh.BlockNumber, EthLatestBlockNumber) + s.Nil(bnh.BlockHash) }, true, }, @@ -94,9 +135,9 @@ func TestUnmarshalBlockNumberOrHash(t *testing.T) { err := bnh.UnmarshalJSON(tc.input) tc.malleate() if tc.expPass { - require.NoError(t, err) + s.NoError(err) } else { - require.Error(t, err) + s.Error(err) } } } diff --git a/eth/rpc/types/query_client.go b/eth/rpc/types/query_client.go index d7e5b722a..d0baa0b4f 100644 --- a/eth/rpc/types/query_client.go +++ b/eth/rpc/types/query_client.go @@ -23,6 +23,11 @@ type QueryClient struct { } // NewQueryClient creates a new gRPC query client +// +// TODO:🔗 https://github.com/NibiruChain/nibiru/issues/1857 +// test(eth): Test GetProof (rpc/types/query_client.go) in a similar manner to +// cosmos-sdk/client/rpc/rpc_test.go using a network after EVM is wired into the +// app keepers: func NewQueryClient(clientCtx client.Context) *QueryClient { return &QueryClient{ ServiceClient: tx.NewServiceClient(clientCtx), diff --git a/eth/rpc/types/rpc.go b/eth/rpc/types/rpc.go index e15a13f74..b09b2f5f5 100644 --- a/eth/rpc/types/rpc.go +++ b/eth/rpc/types/rpc.go @@ -24,8 +24,9 @@ import ( gethparams "github.com/ethereum/go-ethereum/params" ) -// ErrExceedBlockGasLimit defines the error message when tx execution exceeds the block gas limit. -// The tx fee is deducted in ante handler, so it shouldn't be ignored in JSON-RPC API. +// ErrExceedBlockGasLimit defines the error message when tx execution exceeds the +// block gas limit. The tx fee is deducted in ante handler, so it shouldn't be +// ignored in JSON-RPC API. const ErrExceedBlockGasLimit = "out of gas in location: block gas meter; gasWanted:" // ErrStateDBCommit defines the error message when commit after executing EVM