Skip to content

Commit 0ca861f

Browse files
feat: fix execution verification (#2433)
<!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. NOTE: PR titles should follow semantic commits: https://www.conventionalcommits.org/en/v1.0.0/ --> ## Overview Simpler version of #2377, was reverted in #2394 <!-- Please provide an explanation of the PR, including the appropriate context, background, goal, and rationale. If there is an issue with this information, please provide a tl;dr and link the issue. Ex: Closes #<issue number> --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved block validation to enforce stricter checks on block timestamp and application hash consistency. * Enhanced error messages for validation failures to provide clearer feedback. * **Tests** * Re-enabled tests to verify block timestamp and application hash validation during block processing. * **Refactor** * Simplified transaction execution logic for improved performance by removing unnecessary locking during forkchoice state creation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 620ed4f commit 0ca861f

File tree

3 files changed

+30
-35
lines changed

3 files changed

+30
-35
lines changed

block/manager.go

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -800,17 +800,16 @@ func (m *Manager) execValidate(lastState types.State, header *types.SignedHeader
800800
}
801801

802802
// // Verify that the header's timestamp is strictly greater than the last block's time
803-
// headerTime := header.Time()
804-
// if header.Height() > 1 && lastState.LastBlockTime.After(headerTime) {
805-
// return fmt.Errorf("block time must be strictly increasing: got %v, last block time was %v",
806-
// headerTime.UnixNano(), lastState.LastBlockTime)
807-
// }
808-
809-
// // Validate that the header's AppHash matches the lastState's AppHash
810-
// // Note: Assumes deferred execution
811-
// if !bytes.Equal(header.AppHash, lastState.AppHash) {
812-
// return fmt.Errorf("app hash mismatch: expected %x, got %x", lastState.AppHash, header.AppHash)
813-
// }
803+
headerTime := header.Time()
804+
if header.Height() > 1 && lastState.LastBlockTime.After(headerTime) {
805+
return fmt.Errorf("block time must be strictly increasing: got %v, last block time was %v",
806+
headerTime, lastState.LastBlockTime)
807+
}
808+
809+
// AppHash should match the last state's AppHash
810+
if !bytes.Equal(header.AppHash, lastState.AppHash) {
811+
return fmt.Errorf("appHash mismatch in delayed execution mode: expected %x, got %x", lastState.AppHash, header.AppHash)
812+
}
814813

815814
return nil
816815
}

block/manager_test.go

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -575,28 +575,26 @@ func TestManager_execValidate(t *testing.T) {
575575
require.NoError(err)
576576
})
577577

578-
// TODO: https://github.com/rollkit/rollkit/issues/2250
579-
580-
// t.Run("non-monotonic block time with height > 1", func(t *testing.T) {
581-
// state, header, data, privKey := makeValid()
582-
// state.LastBlockTime = time.Now().Add(time.Minute)
583-
// state.LastBlockHeight = 1
584-
// header.BaseHeader.Height = state.LastBlockHeight + 1
585-
// data.Metadata.Height = state.LastBlockHeight + 1
586-
// signer, err := noopsigner.NewNoopSigner(privKey)
587-
// require.NoError(err)
588-
// header.Signature, err = types.GetSignature(header.Header, signer)
589-
// require.NoError(err)
590-
// err = m.execValidate(state, header, data)
591-
// require.ErrorContains(err, "block time must be strictly increasing")
592-
// })
593-
594-
// t.Run("app hash mismatch", func(t *testing.T) {
595-
// state, header, data, _ := makeValid()
596-
// state.AppHash = []byte("different")
597-
// err := m.execValidate(state, header, data)
598-
// require.ErrorContains(err, "app hash mismatch")
599-
// })
578+
t.Run("non-monotonic block time with height > 1", func(t *testing.T) {
579+
state, header, data, privKey := makeValid()
580+
state.LastBlockTime = time.Now().Add(time.Minute)
581+
state.LastBlockHeight = 1
582+
header.BaseHeader.Height = state.LastBlockHeight + 1
583+
data.Metadata.Height = state.LastBlockHeight + 1
584+
signer, err := noopsigner.NewNoopSigner(privKey)
585+
require.NoError(err)
586+
header.Signature, err = types.GetSignature(header.Header, signer)
587+
require.NoError(err)
588+
err = m.execValidate(state, header, data)
589+
require.ErrorContains(err, "block time must be strictly increasing")
590+
})
591+
592+
t.Run("app hash mismatch", func(t *testing.T) {
593+
state, header, data, _ := makeValid()
594+
state.AppHash = []byte("different")
595+
err := m.execValidate(state, header, data)
596+
require.ErrorContains(err, "appHash mismatch in delayed execution mode")
597+
})
600598
}
601599

602600
// TestGetterMethods tests simple getter methods for the Manager

execution/evm/execution.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,13 +191,11 @@ func (c *EngineClient) ExecuteTxs(ctx context.Context, txs [][]byte, blockHeight
191191
ts = prevTimestamp + 1 // Subsequent blocks must have a higher timestamp.
192192
}
193193

194-
c.mu.Lock()
195194
args := engine.ForkchoiceStateV1{
196195
HeadBlockHash: prevBlockHash,
197196
SafeBlockHash: prevBlockHash,
198197
FinalizedBlockHash: prevBlockHash,
199198
}
200-
c.mu.Unlock()
201199

202200
// update forkchoice to get the next payload id
203201
var forkchoiceResult engine.ForkChoiceResponse

0 commit comments

Comments
 (0)