Skip to content

Commit

Permalink
fix validateStaticCommitmentState
Browse files Browse the repository at this point in the history
  • Loading branch information
mycodecrafting committed Mar 4, 2025
1 parent 7f60281 commit 22db36b
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 31 deletions.
2 changes: 1 addition & 1 deletion grpc/execution/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ func (s *ExecutionServiceServerV2) UpdateCommitmentState(ctx context.Context, re
s.commitmentUpdateLock.Lock()
defer s.commitmentUpdateLock.Unlock()

if err := validateStaticCommitmentState(req.CommitmentState); err != nil {
if err := validateStaticCommitmentState(req.CommitmentState, s.softAsFirm); err != nil {
log.Error("UpdateCommitmentState called with invalid CommitmentState", "err", err)
return nil, status.Error(codes.InvalidArgument, "CommitmentState is invalid")
}
Expand Down
17 changes: 5 additions & 12 deletions grpc/execution/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func validateStaticExecuteBlockRequest(req *astriaPb.ExecuteBlockRequest) error
}

// `validateStaticCommitment` validates the given commitment without regard to the current state of the system.
func validateStaticCommitmentState(commitmentState *astriaPb.CommitmentState) error {
func validateStaticCommitmentState(commitmentState *astriaPb.CommitmentState, softAsFirm bool) error {
if commitmentState == nil {
return fmt.Errorf("commitment state is nil")
}
Expand All @@ -126,20 +126,13 @@ func validateStaticCommitmentState(commitmentState *astriaPb.CommitmentState) er
if commitmentState.FirmExecutedBlockMetadata == nil {
return fmt.Errorf("FirmExecutedBlockMetadata cannot be nil")
}
if commitmentState.LowestCelestiaSearchHeight == 0 {
return fmt.Errorf("lowest celestia search height of 0 is not valid")
}

if err := validateStaticExecutedBlockMetadata(commitmentState.SoftExecutedBlockMetadata); err != nil {
return fmt.Errorf("soft block invalid: %w", err)
}
if err := validateStaticExecutedBlockMetadata(commitmentState.FirmExecutedBlockMetadata); err != nil {
return fmt.Errorf("firm block invalid: %w", err)
}

// Verify that the firm block is not newer than the soft block
if commitmentState.FirmExecutedBlockMetadata.Number > commitmentState.SoftExecutedBlockMetadata.Number {
return fmt.Errorf("FirmExecutedBlockMetadata number cannot be greater than SoftExecutedBlockMetadata number")
if !softAsFirm {
if err := validateStaticExecutedBlockMetadata(commitmentState.FirmExecutedBlockMetadata); err != nil {
return fmt.Errorf("firm block invalid: %w", err)
}
}

return nil
Expand Down
22 changes: 4 additions & 18 deletions grpc/execution/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func TestValidateStaticCommitmentState(t *testing.T) {
LowestCelestiaSearchHeight: 100,
}

err := validateStaticCommitmentState(validState)
err := validateStaticCommitmentState(validState, false)
require.Nil(t, err, "Valid CommitmentState should pass validation")

// Test missing SoftExecutedBlockMetadata
Expand All @@ -242,7 +242,7 @@ func TestValidateStaticCommitmentState(t *testing.T) {
FirmExecutedBlockMetadata: validState.FirmExecutedBlockMetadata,
LowestCelestiaSearchHeight: 100,
}
err = validateStaticCommitmentState(invalidState1)
err = validateStaticCommitmentState(invalidState1, false)
require.NotNil(t, err, "CommitmentState without SoftExecutedBlockMetadata should fail validation")
require.Contains(t, err.Error(), "SoftExecutedBlockMetadata cannot be nil", "Error should mention SoftExecutedBlockMetadata")

Expand All @@ -252,7 +252,7 @@ func TestValidateStaticCommitmentState(t *testing.T) {
FirmExecutedBlockMetadata: nil,
LowestCelestiaSearchHeight: 100,
}
err = validateStaticCommitmentState(invalidState2)
err = validateStaticCommitmentState(invalidState2, false)
require.NotNil(t, err, "CommitmentState without FirmExecutedBlockMetadata should fail validation")
require.Contains(t, err.Error(), "FirmExecutedBlockMetadata cannot be nil", "Error should mention FirmExecutedBlockMetadata")

Expand All @@ -267,23 +267,9 @@ func TestValidateStaticCommitmentState(t *testing.T) {
FirmExecutedBlockMetadata: validState.FirmExecutedBlockMetadata,
LowestCelestiaSearchHeight: 100,
}
err = validateStaticCommitmentState(invalidState3)
err = validateStaticCommitmentState(invalidState3, false)
require.NotNil(t, err, "CommitmentState with invalid SoftExecutedBlockMetadata should fail validation")

// Test firm block newer than soft block
invalidState4 := &astriaPb.CommitmentState{
SoftExecutedBlockMetadata: validState.SoftExecutedBlockMetadata,
FirmExecutedBlockMetadata: &astriaPb.ExecutedBlockMetadata{
Number: 11, // Higher than soft block
Hash: "0xabcdef",
ParentHash: "0xfedcba",
Timestamp: &timestamppb.Timestamp{Seconds: 1234567880},
},
LowestCelestiaSearchHeight: 100,
}
err = validateStaticCommitmentState(invalidState4)
require.NotNil(t, err, "CommitmentState with firm block newer than soft block should fail validation")
require.Contains(t, err.Error(), "FirmExecutedBlockMetadata number", "Error should mention FirmExecutedBlockMetadata number")
}

func TestValidateStaticExecutedBlockMetadata(t *testing.T) {
Expand Down

0 comments on commit 22db36b

Please sign in to comment.