Skip to content

Commit

Permalink
fix: refactor handling of hash copy copy logic.
Browse files Browse the repository at this point in the history
Previously, hashes was not copied properly because of nil destination for copy function.

Replaced manual slice copying with a dedicated `copyHash` function for cleaner and reusable code. Updated related assertions and method calls to align with the new approach, improving code readability and maintainability.
  • Loading branch information
tzdybal committed Feb 19, 2025
1 parent bd2b4ce commit 191d5ed
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 16 deletions.
6 changes: 2 additions & 4 deletions proxy/grpc/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ func (c *Client) InitChain(ctx context.Context, genesisTime time.Time, initialHe
return types.Hash{}, 0, err
}

var stateRoot types.Hash
copy(stateRoot[:], resp.StateRoot)
stateRoot := copyHash(resp.StateRoot)

return stateRoot, resp.MaxBytes, nil
}
Expand Down Expand Up @@ -99,8 +98,7 @@ func (c *Client) ExecuteTxs(ctx context.Context, txs []types.Tx, blockHeight uin
return types.Hash{}, 0, err
}

var updatedStateRoot types.Hash
copy(updatedStateRoot[:], resp.UpdatedStateRoot)
updatedStateRoot := copyHash(resp.UpdatedStateRoot)

return updatedStateRoot, resp.MaxBytes, nil
}
Expand Down
8 changes: 3 additions & 5 deletions proxy/grpc/client_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,8 @@ func TestClientServer(t *testing.T) {
chainID := "test-chain"

// initialize a new Hash with a fixed size
expectedStateRoot := make([]byte, 32)
expectedStateRoot := types.Hash(make([]byte, 32))
copy(expectedStateRoot, []byte{1, 2, 3})
var stateRootHash types.Hash
copy(stateRootHash[:], expectedStateRoot)

expectedMaxBytes := uint64(1000000)

Expand All @@ -68,12 +66,12 @@ 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()

stateRoot, maxBytes, err := client.InitChain(context.TODO(), 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
3 changes: 1 addition & 2 deletions proxy/grpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ func (s *Server) ExecuteTxs(ctx context.Context, req *pb.ExecuteTxsRequest) (*pb
txs[i] = tx
}

var prevStateRoot types.Hash
copy(prevStateRoot[:], req.PrevStateRoot)
prevStateRoot := copyHash(req.PrevStateRoot)

updatedStateRoot, maxBytes, err := s.exec.ExecuteTxs(
ctx,
Expand Down
9 changes: 9 additions & 0 deletions proxy/grpc/util.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package grpc

import "github.com/rollkit/go-execution/types"

func copyHash(src []byte) types.Hash {
dst := make([]byte, len(src))
copy(dst, src)
return dst
}
11 changes: 6 additions & 5 deletions test/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ func (s *ExecutorSuite) TestExecuteTxs() {

stateRoot, maxBytes, err := s.Exec.ExecuteTxs(context.TODO(), txs, initialHeight+1, genesisTime.Add(time.Second), genesisStateRoot)
s.Require().NoError(err)
s.NotEqual(types.Hash{}, stateRoot)
s.NotEqual(genesisStateRoot, stateRoot)
s.Greater(maxBytes, uint64(0))
s.Require().NotEmpty(stateRoot)
s.Require().NotEqualValues(genesisStateRoot, stateRoot)
s.Require().Greater(maxBytes, uint64(0))
}

// TestSetFinal tests SetFinal method.
Expand All @@ -73,8 +73,8 @@ func (s *ExecutorSuite) TestSetFinal() {
err := s.Exec.SetFinal(context.TODO(), 1)
s.Require().Error(err)

_, _, _, _ = s.initChain(context.TODO())
_, _, err = s.Exec.ExecuteTxs(context.TODO(), nil, 2, time.Now(), types.Hash("test state"))
_, height, stateRoot, _ := s.initChain(context.TODO())
_, _, err = s.Exec.ExecuteTxs(context.TODO(), nil, height+1, time.Now(), stateRoot)
s.Require().NoError(err)
err = s.Exec.SetFinal(context.TODO(), 2)
s.Require().NoError(err)
Expand Down Expand Up @@ -107,5 +107,6 @@ func (s *ExecutorSuite) initChain(ctx context.Context) (time.Time, uint64, types

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

0 comments on commit 191d5ed

Please sign in to comment.