Skip to content
This repository was archived by the owner on Apr 2, 2025. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions proxy/grpc/client_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,6 @@ func TestClientServer(t *testing.T) {
// initialize a new Hash with a fixed size
expectedStateRoot := types.Hash(make([]byte, 32))
copy(expectedStateRoot, []byte{1, 2, 3})
stateRootHash := types.Hash(make([]byte, 32))
copy(stateRootHash[:], expectedStateRoot)
assert.Equal(t, expectedStateRoot, stateRootHash[:])

expectedMaxBytes := uint64(1000000)

Expand All @@ -69,14 +66,14 @@ func TestClientServer(t *testing.T) {
expectedTime := time.Unix(unixTime, 0).UTC()

mockExec.On("InitChain", mock.Anything, expectedTime, initialHeight, chainID).
Return(stateRootHash, expectedMaxBytes, nil).Once()
Return(expectedStateRoot, expectedMaxBytes, nil).Once()

ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
defer cancel()
stateRoot, maxBytes, err := client.InitChain(ctx, genesisTime, initialHeight, chainID)

require.NoError(t, err)
assert.Equal(t, stateRootHash, stateRoot)
assert.Equal(t, expectedStateRoot, stateRoot)
assert.Equal(t, expectedMaxBytes, maxBytes)
mockExec.AssertExpectations(t)
})
Expand Down
32 changes: 29 additions & 3 deletions test/dummy.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package test

import (
"bytes"
"crypto/rand"
"fmt"

"context"
"crypto/sha512"
"regexp"
Expand Down Expand Up @@ -70,12 +73,30 @@ func (e *DummyExecutor) GetTxs(context.Context) ([]types.Tx, error) {
return txs, nil
}

// InjectTx adds a transaction to the internal list of injected transactions in the DummyExecutor instance.
func (e *DummyExecutor) InjectTx(tx types.Tx) {
// GetRandomTxs generates a slice of n random transactions (types.Tx), each containing 100 random bytes.
func (e *DummyExecutor) GetRandomTxs(n int) []types.Tx {
txs := make([]types.Tx, n)
for i := 0; i < n; i++ {
txs[i] = mustGetRandomBytes(100)
}
return txs
}

// InjectTxs adds a slice of transactions to the internal list of injected transactions in a thread-safe manner.
func (e *DummyExecutor) InjectTxs(txs []types.Tx) error {
e.mu.Lock()
defer e.mu.Unlock()

e.injectedTxs = append(e.injectedTxs, tx)
e.injectedTxs = append(e.injectedTxs, txs...)
return nil
}
func mustGetRandomBytes(n int) []byte {
b := make([]byte, n)
_, err := rand.Read(b)
if err != nil {
panic(fmt.Errorf("failed to generate random bytes: %w", err))
}
return b
}

// ExecuteTxs simulate execution of transactions.
Expand Down Expand Up @@ -105,6 +126,11 @@ func (e *DummyExecutor) ExecuteTxs(ctx context.Context, txs []types.Tx, blockHei
}
}

if len(txs) == 0 {
e.pendingRoots[blockHeight] = prevStateRoot
return prevStateRoot, e.maxBytes, nil
}

hash := sha512.New()
hash.Write(prevStateRoot)
for _, tx := range txs {
Expand Down
22 changes: 13 additions & 9 deletions test/dummy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,23 @@ func TestDummySuite(t *testing.T) {
func (s *DummyTestSuite) TestTxRemoval() {
t := s.T()
exec := NewDummyExecutor()
tx1 := types.Tx([]byte{1, 2, 3})
tx2 := types.Tx([]byte{3, 2, 1})

exec.InjectTx(tx1)
exec.InjectTx(tx2)
// Generate random transactions using GetRandomTxs
randomTxs := exec.GetRandomTxs(2)

// first execution of GetTxs - nothing special
// Inject the random transactions into the executor
err := exec.InjectTxs(randomTxs)
require.NoError(t, err)

tx1 := randomTxs[0]
tx2 := randomTxs[1]

// Retrieve transactions and verify
txs, err := exec.GetTxs(context.Background())
require.NoError(t, err)
require.Len(t, txs, 2)
require.Contains(t, txs, tx1)
require.Contains(t, txs, tx2)

require.Contains(t, txs, randomTxs[0])
require.Contains(t, txs, randomTxs[1])
// ExecuteTxs was not called, so 2 txs should still be returned
txs, err = exec.GetTxs(context.Background())
require.NoError(t, err)
Expand Down Expand Up @@ -210,7 +214,7 @@ func (s *DummyTestSuite) TestGetTxsWithConcurrency() {
defer wg.Done()
for j := 0; j < txsPerGoroutine; j++ {
tx := types.Tx([]byte(fmt.Sprintf("tx-%d-%d", id, j)))
s.TxInjector.InjectTx(tx)
s.Require().NoError(s.TxInjector.InjectTxs([]types.Tx{tx}))
}
}(i)
}
Expand Down
144 changes: 107 additions & 37 deletions test/suite.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package test

import (
"bytes"
"context"
"fmt"
"time"

"github.com/stretchr/testify/suite"
Expand All @@ -19,7 +21,8 @@

// TxInjector provides an interface for injecting transactions into a test suite.
type TxInjector interface {
InjectTx(tx types.Tx)
GetRandomTxs(n int) []types.Tx
InjectTxs(tx []types.Tx) error
}

// TestInitChain tests InitChain method.
Expand All @@ -41,20 +44,25 @@
func (s *ExecutorSuite) TestGetTxs() {
s.skipIfInjectorNotSet()

tx1 := types.Tx("tx1")
tx2 := types.Tx("tx2")

s.TxInjector.InjectTx(tx1)
s.TxInjector.InjectTx(tx2)

ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
defer cancel()

// try to get transactions without injecting any
txs, err := s.Exec.GetTxs(ctx)
s.Require().NoError(err)
s.Require().Empty(txs)

// inject two txs and retrieve them
// inject two txs and retrieve them using GetRandomTxs and InjectTxs
randomTxs := s.TxInjector.GetRandomTxs(2) // Retrieve 2 random transactions
err = s.TxInjector.InjectTxs(randomTxs) // Inject the transactions into the state
s.Require().NoError(err)

txs, err = s.Exec.GetTxs(ctx) // Retrieve transactions from the executor
s.Require().NoError(err)
s.Require().Len(txs, 2)
s.Require().Contains(txs, tx1)
s.Require().Contains(txs, tx2)
s.Require().Contains(txs, randomTxs[0])
s.Require().Contains(txs, randomTxs[1])
}

func (s *ExecutorSuite) skipIfInjectorNotSet() {
Expand All @@ -65,18 +73,51 @@

// TestExecuteTxs tests ExecuteTxs method.
func (s *ExecutorSuite) TestExecuteTxs() {
txs := []types.Tx{[]byte("tx1"), []byte("tx2")}
blockHeight := uint64(1)
timestamp := time.Now().UTC()
prevStateRoot := types.Hash{1, 2, 3}
s.skipIfInjectorNotSet()

ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
cases := []struct {
name string
txs []types.Tx
stateRootChanged bool
}{
{
name: "empty txs",
txs: []types.Tx{},
stateRootChanged: false,
},
{
name: "nil txs",
txs: nil,
stateRootChanged: false,
},
{
name: "two txs",
txs: s.TxInjector.GetRandomTxs(2),
stateRootChanged: true,
},
}

ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()

stateRoot, maxBytes, err := s.Exec.ExecuteTxs(ctx, txs, blockHeight, timestamp, prevStateRoot)
s.Require().NoError(err)
s.NotEqual(types.Hash{}, stateRoot)
s.Greater(maxBytes, uint64(0))
genesisTime, lastStateRoot, _ := s.initChain(ctx, uint64(1))

for i, c := range cases {
s.Run(c.name, func() {
err := s.TxInjector.InjectTxs(c.txs)
s.Require().NoError(err)
txs, _ := s.Exec.GetTxs(ctx)
fmt.Println(len(txs))
stateRoot, maxBytes, err := s.Exec.ExecuteTxs(ctx, c.txs, uint64(i+1), genesisTime.Add(time.Duration(i)*time.Second), lastStateRoot)

Check failure on line 111 in test/suite.go

View workflow job for this annotation

GitHub Actions / lint / golangci-lint

G115: integer overflow conversion int -> uint64 (gosec)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix potential integer overflow in type conversion

There's a potential integer overflow when converting i+1 from int to uint64.

-			stateRoot, maxBytes, err := s.Exec.ExecuteTxs(ctx, c.txs, uint64(i+1), genesisTime.Add(time.Duration(i)*time.Second), lastStateRoot)
+			stateRoot, maxBytes, err := s.Exec.ExecuteTxs(ctx, c.txs, uint64(i)+1, genesisTime.Add(time.Duration(i)*time.Second), lastStateRoot)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
stateRoot, maxBytes, err := s.Exec.ExecuteTxs(ctx, c.txs, uint64(i+1), genesisTime.Add(time.Duration(i)*time.Second), lastStateRoot)
stateRoot, maxBytes, err := s.Exec.ExecuteTxs(ctx, c.txs, uint64(i)+1, genesisTime.Add(time.Duration(i)*time.Second), lastStateRoot)
🧰 Tools
🪛 GitHub Check: lint / golangci-lint

[failure] 111-111:
G115: integer overflow conversion int -> uint64 (gosec)

🪛 golangci-lint (1.62.2)

[high] 111-111: G115: integer overflow conversion int -> uint64

(gosec)

s.Require().NoError(err)
s.Require().NotEmpty(stateRoot)
s.Assert().Equal(c.stateRootChanged, !bytes.Equal(lastStateRoot, stateRoot))
s.Require().Greater(maxBytes, uint64(0))
lastStateRoot = stateRoot
s.Exec.SetFinal(ctx, uint64(i+1))

Check failure on line 117 in test/suite.go

View workflow job for this annotation

GitHub Actions / lint / golangci-lint

Error return value of `s.Exec.SetFinal` is not checked (errcheck)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Error return value from SetFinal is not checked

The error returned by s.Exec.SetFinal is not being checked, which could mask failures in the test.

-			s.Exec.SetFinal(ctx, uint64(i+1))
+			err = s.Exec.SetFinal(ctx, uint64(i+1))
+			s.Require().NoError(err)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
s.Exec.SetFinal(ctx, uint64(i+1))
err = s.Exec.SetFinal(ctx, uint64(i+1))
s.Require().NoError(err)
🧰 Tools
🪛 GitHub Check: lint / golangci-lint

[failure] 117-117:
Error return value of s.Exec.SetFinal is not checked (errcheck)

🪛 golangci-lint (1.62.2)

117-117: Error return value of s.Exec.SetFinal is not checked

(errcheck)

🪛 GitHub Actions: CI and Release

[error] 117-117: Error return value of s.Exec.SetFinal is not checked (errcheck)

time.Sleep(3 * time.Second)
})
}
Comment on lines +78 to +120
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

❓ Verification inconclusive

Table-driven testing approach enhances test coverage and readability

The refactoring to a table-driven test structure is a significant improvement that makes the test cases more organized and maintainable. However, there's an unchecked error return from SetFinal on line 117.

-			s.Exec.SetFinal(ctx, uint64(i+1))
+			err = s.Exec.SetFinal(ctx, uint64(i+1))
+			s.Require().NoError(err)

Additionally, there's a potential integer overflow warning on line 111 when converting i+1 to uint64:

-			stateRoot, maxBytes, err := s.Exec.ExecuteTxs(ctx, c.txs, uint64(i+1), genesisTime.Add(time.Duration(i)*time.Second), lastStateRoot)
+			stateRoot, maxBytes, err := s.Exec.ExecuteTxs(ctx, c.txs, uint64(i)+1, genesisTime.Add(time.Duration(i)*time.Second), lastStateRoot)

Action Required: Fix error handling and integer conversion issues in the table-driven test

  • Update the call to SetFinal to capture and check the error return:

    -			s.Exec.SetFinal(ctx, uint64(i+1))
    +			err = s.Exec.SetFinal(ctx, uint64(i+1))
    +			s.Require().NoError(err)
  • Revise the conversion of the loop index when calling ExecuteTxs to prevent any potential integer overflow warning. For instance, change:

    -			stateRoot, maxBytes, err := s.Exec.ExecuteTxs(ctx, c.txs, uint64(i+1), genesisTime.Add(time.Duration(i)*time.Second), lastStateRoot)
    +			stateRoot, maxBytes, err := s.Exec.ExecuteTxs(ctx, c.txs, uint64(i)+1, genesisTime.Add(time.Duration(i)*time.Second), lastStateRoot)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cases := []struct {
name string
txs []types.Tx
stateRootChanged bool
}{
{
name: "empty txs",
txs: []types.Tx{},
stateRootChanged: false,
},
{
name: "nil txs",
txs: nil,
stateRootChanged: false,
},
{
name: "two txs",
txs: s.TxInjector.GetRandomTxs(2),
stateRootChanged: true,
},
}
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
stateRoot, maxBytes, err := s.Exec.ExecuteTxs(ctx, txs, blockHeight, timestamp, prevStateRoot)
s.Require().NoError(err)
s.NotEqual(types.Hash{}, stateRoot)
s.Greater(maxBytes, uint64(0))
genesisTime, lastStateRoot, _ := s.initChain(ctx, uint64(1))
for i, c := range cases {
s.Run(c.name, func() {
err := s.TxInjector.InjectTxs(c.txs)
s.Require().NoError(err)
txs, _ := s.Exec.GetTxs(ctx)
fmt.Println(len(txs))
stateRoot, maxBytes, err := s.Exec.ExecuteTxs(ctx, c.txs, uint64(i+1), genesisTime.Add(time.Duration(i)*time.Second), lastStateRoot)
s.Require().NoError(err)
s.Require().NotEmpty(stateRoot)
s.Assert().Equal(c.stateRootChanged, !bytes.Equal(lastStateRoot, stateRoot))
s.Require().Greater(maxBytes, uint64(0))
lastStateRoot = stateRoot
s.Exec.SetFinal(ctx, uint64(i+1))
time.Sleep(3 * time.Second)
})
}
cases := []struct {
name string
txs []types.Tx
stateRootChanged bool
}{
{
name: "empty txs",
txs: []types.Tx{},
stateRootChanged: false,
},
{
name: "nil txs",
txs: nil,
stateRootChanged: false,
},
{
name: "two txs",
txs: s.TxInjector.GetRandomTxs(2),
stateRootChanged: true,
},
}
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
genesisTime, lastStateRoot, _ := s.initChain(ctx, uint64(1))
for i, c := range cases {
s.Run(c.name, func() {
err := s.TxInjector.InjectTxs(c.txs)
s.Require().NoError(err)
txs, _ := s.Exec.GetTxs(ctx)
fmt.Println(len(txs))
stateRoot, maxBytes, err := s.Exec.ExecuteTxs(ctx, c.txs, uint64(i)+1, genesisTime.Add(time.Duration(i)*time.Second), lastStateRoot)
s.Require().NoError(err)
s.Require().NotEmpty(stateRoot)
s.Assert().Equal(c.stateRootChanged, !bytes.Equal(lastStateRoot, stateRoot))
s.Require().Greater(maxBytes, uint64(0))
lastStateRoot = stateRoot
err = s.Exec.SetFinal(ctx, uint64(i+1))
s.Require().NoError(err)
time.Sleep(3 * time.Second)
})
}
🧰 Tools
🪛 GitHub Check: lint / golangci-lint

[failure] 117-117:
Error return value of s.Exec.SetFinal is not checked (errcheck)


[failure] 111-111:
G115: integer overflow conversion int -> uint64 (gosec)

🪛 golangci-lint (1.62.2)

117-117: Error return value of s.Exec.SetFinal is not checked

(errcheck)


[high] 111-111: G115: integer overflow conversion int -> uint64

(gosec)

🪛 GitHub Actions: CI and Release

[error] 117-117: Error return value of s.Exec.SetFinal is not checked (errcheck)

}

// TestSetFinal tests SetFinal method.
Expand All @@ -85,43 +126,72 @@
defer cancel()

// finalizing invalid height must return error
err := s.Exec.SetFinal(ctx, 1)
err := s.Exec.SetFinal(ctx, 7)
s.Require().Error(err)

ctx2, cancel2 := context.WithTimeout(context.Background(), 3*time.Second)
defer cancel2()
_, _, err = s.Exec.ExecuteTxs(ctx2, nil, 2, time.Now(), types.Hash("test state"))
initialHeight := uint64(1)
_, stateRoot, _ := s.initChain(ctx, initialHeight)
_, _, err = s.Exec.ExecuteTxs(ctx, nil, initialHeight, time.Now(), stateRoot)
s.Require().NoError(err)

ctx3, cancel3 := context.WithTimeout(context.Background(), 3*time.Second)
defer cancel3()
err = s.Exec.SetFinal(ctx3, 2)
err = s.Exec.SetFinal(ctx, initialHeight)
s.Require().NoError(err)
}

// TestMultipleBlocks is a basic test ensuring that all API methods used together can be used to produce multiple blocks.
// TestMultipleBlocks is a basic test ensuring that all API methods used together can be used to produce multiple fresh blocks.
func (s *ExecutorSuite) TestMultipleBlocks() {
genesisTime := time.Now().UTC()
initialHeight := uint64(1)
chainID := "test-chain"
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
defer cancel()

stateRoot, maxBytes, err := s.Exec.InitChain(ctx, genesisTime, initialHeight, chainID)
s.Require().NoError(err)
s.NotEqual(types.Hash{}, stateRoot)
s.Greater(maxBytes, uint64(0))
initialHeight := uint64(1)
genesisTime, prevStateRoot, _ := s.initChain(ctx, initialHeight)

for i := initialHeight; i <= 10; i++ {
err := s.TxInjector.InjectTxs(s.TxInjector.GetRandomTxs(2))
s.Require().NoError(err)
txs, err := s.Exec.GetTxs(ctx)
s.Require().NoError(err)

blockTime := genesisTime.Add(time.Duration(i+1) * time.Second) //nolint:gosec
stateRoot, maxBytes, err = s.Exec.ExecuteTxs(ctx, txs, i, blockTime, stateRoot)
stateRoot, maxBytes, err := s.Exec.ExecuteTxs(ctx, txs, i, blockTime, prevStateRoot)
s.Require().NoError(err)
s.Require().NotZero(maxBytes)
s.Assert().NotEqual(prevStateRoot, stateRoot)

prevStateRoot = stateRoot

err = s.Exec.SetFinal(ctx, i)
s.Require().NoError(err)
}
}

// TestSyncScenario is a basic test ensuring that all API methods used together can be used to sync multiple blocks without injecting transactions to mempool.
func (s *ExecutorSuite) TestSyncScenario() {
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
defer cancel()
initialHeight := uint64(1)
genesisTime, prevStateRoot, _ := s.initChain(ctx, initialHeight)

for i := initialHeight; i <= 10; i++ {
txs := s.TxInjector.GetRandomTxs(2)

blockTime := genesisTime.Add(time.Duration(i+1) * time.Second) //nolint:gosec
stateRoot, maxBytes, err := s.Exec.ExecuteTxs(ctx, txs, i, blockTime, prevStateRoot)
s.Require().NoError(err)
s.Require().NotZero(maxBytes)
s.Assert().NotEqual(prevStateRoot, stateRoot)

prevStateRoot = stateRoot

err = s.Exec.SetFinal(ctx, i)
s.Require().NoError(err)
}
}

func (s *ExecutorSuite) initChain(ctx context.Context, initialHeight uint64) (time.Time, types.Hash, uint64) {
genesisTime := time.Now().UTC()
chainID := fmt.Sprintf("test-chain-%d", initialHeight)

stateRoot, maxBytes, err := s.Exec.InitChain(ctx, genesisTime, initialHeight, chainID)
s.Require().NoError(err)
s.Require().NotEmpty(stateRoot)
return genesisTime, stateRoot, maxBytes
}
Loading