From a11d1181b7fda10d65bca50492f612dcdc4c0ae4 Mon Sep 17 00:00:00 2001 From: Jiankun Yang <110372666+0xyjk@users.noreply.github.com> Date: Fri, 3 May 2024 14:34:34 +0800 Subject: [PATCH 1/5] fix: plasma finalize --- op-plasma/dastate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/op-plasma/dastate.go b/op-plasma/dastate.go index c6fee4800af9..ab6139e28a81 100644 --- a/op-plasma/dastate.go +++ b/op-plasma/dastate.go @@ -172,7 +172,7 @@ func (s *State) GetResolvedInput(key []byte) ([]byte, error) { // it returns an error to signal that a derivation pipeline reset is required. func (s *State) ExpireChallenges(bn uint64) (uint64, error) { var err error - for s.activeComms.Len() > 0 && s.activeComms[0].expiresAt <= bn && s.activeComms[0].blockNumber > s.finalized { + for s.activeComms.Len() > 0 && s.activeComms[0].expiresAt <= bn && s.activeComms[0].blockNumber >= s.finalized { // move from the active to the expired queue c := heap.Pop(&s.activeComms).(*Commitment) heap.Push(&s.expiredComms, c) From 0ea664f2a02f5f8e1ef1bc4dc8ae07d2665fa7d3 Mon Sep 17 00:00:00 2001 From: Jiankun Yang <110372666+0xyjk@users.noreply.github.com> Date: Sat, 11 May 2024 10:54:10 +0800 Subject: [PATCH 2/5] fix: advance l1 origin --- op-plasma/damgr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/op-plasma/damgr.go b/op-plasma/damgr.go index 533532b76b17..755ce74413f3 100644 --- a/op-plasma/damgr.go +++ b/op-plasma/damgr.go @@ -240,7 +240,7 @@ func (d *DA) isExpired(bn uint64) bool { // trigger a derivation reset. func (d *DA) AdvanceL1Origin(ctx context.Context, l1 L1Fetcher, block eth.BlockID) error { // do not repeat for the same origin - if block.Number <= d.origin.Number { + if block.Number < d.origin.Number { return nil } // sync challenges for the given block ID From 221859c8af2fda2f8db7c03a6ce03db6bc518a2e Mon Sep 17 00:00:00 2001 From: Jiankun Yang <110372666+0xyjk@users.noreply.github.com> Date: Sat, 11 May 2024 12:04:39 +0800 Subject: [PATCH 3/5] test: add test for advance l1 origin --- op-plasma/damgr_test.go | 49 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/op-plasma/damgr_test.go b/op-plasma/damgr_test.go index 5a37badc19ca..df10e01cffaf 100644 --- a/op-plasma/damgr_test.go +++ b/op-plasma/damgr_test.go @@ -21,6 +21,49 @@ func RandomData(rng *rand.Rand, size int) []byte { return out } +// TestAdvanceL1Origin test advance l1 origin when there are multiple commitments in the same block +func TestAdvanceL1Origin(t *testing.T) { + var ( + logger = testlog.Logger(t, log.LevelDebug) + ctx = context.Background() + bn uint64 = 1 + l1F = &mockL1Fetcher{} + err error + ) + l1F.ExpectFetchReceipts(common.HexToHash(""), nil, types.Receipts{}, nil) + l1F.ExpectL1BlockRefByNumber(bn, eth.L1BlockRef{Number: bn}, nil) + + // simulate one block has multiple commitments + state := NewState(logger, &NoopMetrics{}) + state.SetInputCommitment([]byte("0x1"), bn, 0) + state.SetInputCommitment([]byte("0x2"), bn, 0) + state.SetInputCommitment([]byte("0x3"), bn, 0) + + da := NewPlasmaDAWithState(logger, Config{}, NewMockDAClient(logger), &NoopMetrics{}, state) + err = da.AdvanceL1Origin(ctx, l1F, eth.BlockID{Number: bn}) + require.NoError(t, err) + require.Equal(t, da.state.finalized, bn) + + // simulate another block with no commitments, finalize should not moved + bn++ + l1F.ExpectFetchReceipts(common.HexToHash(""), nil, types.Receipts{}, nil) + l1F.ExpectL1BlockRefByNumber(bn, eth.L1BlockRef{Number: bn}, nil) + err = da.AdvanceL1Origin(ctx, l1F, eth.BlockID{Number: bn}) + require.NoError(t, err) + require.Equal(t, da.state.finalized, bn-1) + + // simulate another block with multiple commitments, finalize should moved + bn++ + l1F.ExpectFetchReceipts(common.HexToHash(""), nil, types.Receipts{}, nil) + l1F.ExpectL1BlockRefByNumber(bn, eth.L1BlockRef{Number: bn}, nil) + state.SetInputCommitment([]byte("0x4"), bn, 0) + state.SetInputCommitment([]byte("0x5"), bn, 0) + state.SetInputCommitment([]byte("0x6"), bn, 0) + err = da.AdvanceL1Origin(ctx, l1F, eth.BlockID{Number: bn}) + require.NoError(t, err) + require.Equal(t, da.state.finalized, bn) +} + // TestDAChallengeState is a simple test with small values to verify the finalized head logic func TestDAChallengeState(t *testing.T) { logger := testlog.Logger(t, log.LvlDebug) @@ -269,7 +312,7 @@ func (m *mockL1Fetcher) InfoAndTxsByHash(ctx context.Context, hash common.Hash) } func (m *mockL1Fetcher) ExpectInfoAndTxsByHash(hash common.Hash, info eth.BlockInfo, transactions types.Transactions, err error) { - m.Mock.On("InfoAndTxsByHash", hash).Once().Return(info, transactions, err) + m.Mock.On("InfoAndTxsByHash", hash).Return(info, transactions, err) } func (m *mockL1Fetcher) FetchReceipts(ctx context.Context, blockHash common.Hash) (eth.BlockInfo, types.Receipts, error) { @@ -278,7 +321,7 @@ func (m *mockL1Fetcher) FetchReceipts(ctx context.Context, blockHash common.Hash } func (m *mockL1Fetcher) ExpectFetchReceipts(hash common.Hash, info eth.BlockInfo, receipts types.Receipts, err error) { - m.Mock.On("FetchReceipts", hash).Once().Return(&info, receipts, err) + m.Mock.On("FetchReceipts", hash).Return(&info, receipts, err) } func (m *mockL1Fetcher) L1BlockRefByNumber(ctx context.Context, num uint64) (eth.L1BlockRef, error) { @@ -287,7 +330,7 @@ func (m *mockL1Fetcher) L1BlockRefByNumber(ctx context.Context, num uint64) (eth } func (m *mockL1Fetcher) ExpectL1BlockRefByNumber(num uint64, ref eth.L1BlockRef, err error) { - m.Mock.On("L1BlockRefByNumber", num).Once().Return(ref, err) + m.Mock.On("L1BlockRefByNumber", num).Return(ref, err) } func TestFilterInvalidBlockNumber(t *testing.T) { From f3f2d44a8fa7a640dfc0f1778efcaa5ac66bf475 Mon Sep 17 00:00:00 2001 From: Jiankun Yang <110372666+0xyjk@users.noreply.github.com> Date: Tue, 14 May 2024 23:30:43 +0800 Subject: [PATCH 4/5] test: add more test for advance l1 origin --- op-plasma/damgr_test.go | 65 +++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 25 deletions(-) diff --git a/op-plasma/damgr_test.go b/op-plasma/damgr_test.go index df10e01cffaf..b8b0b2e8c506 100644 --- a/op-plasma/damgr_test.go +++ b/op-plasma/damgr_test.go @@ -24,44 +24,59 @@ func RandomData(rng *rand.Rand, size int) []byte { // TestAdvanceL1Origin test advance l1 origin when there are multiple commitments in the same block func TestAdvanceL1Origin(t *testing.T) { var ( - logger = testlog.Logger(t, log.LevelDebug) - ctx = context.Background() - bn uint64 = 1 - l1F = &mockL1Fetcher{} - err error + logger = testlog.Logger(t, log.LevelDebug) + ctx = context.Background() + bn uint64 + challengeWindow uint64 = 1 + l1F = &mockL1Fetcher{} + err error ) - l1F.ExpectFetchReceipts(common.HexToHash(""), nil, types.Receipts{}, nil) - l1F.ExpectL1BlockRefByNumber(bn, eth.L1BlockRef{Number: bn}, nil) - // simulate one block has multiple commitments + l1F.ExpectFetchReceipts(common.HexToHash(""), nil, types.Receipts{}, nil) + // simulate one block has multiple commitments, + // commitments block number are 1 + // finalized should move to 1 when advanced to block number 2 state := NewState(logger, &NoopMetrics{}) - state.SetInputCommitment([]byte("0x1"), bn, 0) - state.SetInputCommitment([]byte("0x2"), bn, 0) - state.SetInputCommitment([]byte("0x3"), bn, 0) + state.SetInputCommitment([]byte("0x1"), 1, challengeWindow) + state.SetInputCommitment([]byte("0x2"), 1, challengeWindow) da := NewPlasmaDAWithState(logger, Config{}, NewMockDAClient(logger), &NoopMetrics{}, state) + bn = 1 + // advance block number 1, finalized should not move due to challenge window = 1 + l1F.ExpectL1BlockRefByNumber(1, eth.L1BlockRef{Number: 1}, nil) + err = da.AdvanceL1Origin(ctx, l1F, eth.BlockID{Number: bn}) + require.NoError(t, err) err = da.AdvanceL1Origin(ctx, l1F, eth.BlockID{Number: bn}) require.NoError(t, err) - require.Equal(t, da.state.finalized, bn) - // simulate another block with no commitments, finalize should not moved - bn++ - l1F.ExpectFetchReceipts(common.HexToHash(""), nil, types.Receipts{}, nil) - l1F.ExpectL1BlockRefByNumber(bn, eth.L1BlockRef{Number: bn}, nil) + bn = 2 + // advance to block number 2, finalized should move to 1 + l1F.ExpectL1BlockRefByNumber(2, eth.L1BlockRef{Number: 2}, nil) err = da.AdvanceL1Origin(ctx, l1F, eth.BlockID{Number: bn}) require.NoError(t, err) - require.Equal(t, da.state.finalized, bn-1) + require.Equal(t, da.state.finalized, uint64(1)) - // simulate another block with multiple commitments, finalize should moved - bn++ - l1F.ExpectFetchReceipts(common.HexToHash(""), nil, types.Receipts{}, nil) - l1F.ExpectL1BlockRefByNumber(bn, eth.L1BlockRef{Number: bn}, nil) - state.SetInputCommitment([]byte("0x4"), bn, 0) - state.SetInputCommitment([]byte("0x5"), bn, 0) - state.SetInputCommitment([]byte("0x6"), bn, 0) + // simulate another block with no commitments, finalize should not moved due to no commitments + bn = 3 + l1F.ExpectL1BlockRefByNumber(3, eth.L1BlockRef{Number: 3}, nil) + err = da.AdvanceL1Origin(ctx, l1F, eth.BlockID{Number: bn}) + require.NoError(t, err) + require.Equal(t, da.state.finalized, uint64(1)) + + // simulate another block with one commitment, finalize should not moved due to challenge window = 1 + bn = 4 + l1F.ExpectL1BlockRefByNumber(4, eth.L1BlockRef{Number: 4}, nil) + state.SetInputCommitment([]byte("0x3"), bn, challengeWindow) + err = da.AdvanceL1Origin(ctx, l1F, eth.BlockID{Number: bn}) + require.NoError(t, err) + require.Equal(t, da.state.finalized, uint64(1)) + + // advance to block number 5, finalized should move to 4 + bn = 5 + l1F.ExpectL1BlockRefByNumber(5, eth.L1BlockRef{Number: 5}, nil) err = da.AdvanceL1Origin(ctx, l1F, eth.BlockID{Number: bn}) require.NoError(t, err) - require.Equal(t, da.state.finalized, bn) + require.Equal(t, da.state.finalized, uint64(4)) } // TestDAChallengeState is a simple test with small values to verify the finalized head logic From 572a7362afc093ef2392b2b3173b9236d52c2a26 Mon Sep 17 00:00:00 2001 From: Jiankun Yang <110372666+0xyjk@users.noreply.github.com> Date: Mon, 20 May 2024 15:23:23 +0800 Subject: [PATCH 5/5] fix: test --- op-service/testutils/mock_eth_client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/op-service/testutils/mock_eth_client.go b/op-service/testutils/mock_eth_client.go index fcb0a60c24b0..ae4a5768195d 100644 --- a/op-service/testutils/mock_eth_client.go +++ b/op-service/testutils/mock_eth_client.go @@ -103,7 +103,7 @@ func (m *MockEthClient) FetchReceipts(ctx context.Context, blockHash common.Hash } func (m *MockEthClient) ExpectFetchReceipts(hash common.Hash, info eth.BlockInfo, receipts types.Receipts, err error) { - m.Mock.On("FetchReceipts", hash).Once().Return(&info, receipts, err) + m.Mock.On("FetchReceipts", hash).Return(&info, receipts, err) } func (m *MockEthClient) GetProof(ctx context.Context, address common.Address, storage []common.Hash, blockTag string) (*eth.AccountResult, error) {