Skip to content

chore: remove extra space in comment #3319

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
box4wangjing wants to merge 1 commit into
base: main
Choose a base branch
from
+3 −3
Open

chore: remove extra space in comment #3319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion block/internal/submitting/submitter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ func TestSubmitter_CacheClearedOnHeightInclusion(t *testing.T) {
//
// Scenario:
// 1. Node runs normally: heights 1–3 are DA-included, height 3 is in-flight
// (submitted to DA but not yet finalized). SetHeaderDAIncluded writes both
// (submitted to DA but not yet finalized). SetHeaderDAIncluded writes both
// the real-hash entry AND the snapshot key.
// 2. Node restarts: a fresh Manager is constructed on the same store.
// RestoreFromStore reads the snapshot and installs placeholder entries
Expand Down
2 changes: 1 addition & 1 deletion pkg/signer/aws/signer_benchmark_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
// BenchmarkKmsSignerSign test kms round trip
// export EVNODE_E2E_GCP_KMS_KEY_NAME=projects/<project-id>/locations/<region>/keyRings/<keyring-name>/cryptoKeys/<key-name>/cryptoKeyVersions/1
// export EVNODE_E2E_GCP_KMS_CREDENTIALS_FILE=...
// go test -v -bench=BenchmarkKmsSignerSign -benchtime=3s -count=10 -run='^$' ./pkg/signer/gcp
// go test -v -bench=BenchmarkKmsSignerSign -benchtime=3s -count=10 -run='^$' ./pkg/signer/gcp
Comment on lines 16 to +18
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Wrong environment variables and test path in benchmark documentation.

The comment documentation has critical errors:

  1. Lines 16-17 reference GCP KMS environment variables (EVNODE_E2E_GCP_KMS_KEY_NAME, EVNODE_E2E_GCP_KMS_CREDENTIALS_FILE), but this is an AWS KMS benchmark test that actually uses EVNODE_E2E_AWS_KMS_KEY_ID (line 20) and AWS region variables (lines 25-29).

  2. Line 18 references the wrong test path ./pkg/signer/gcp, but this file is pkg/signer/aws/signer_benchmark_test.go, so the path should be ./pkg/signer/aws.

Developers following these comments will set the wrong environment variables and target the wrong package.

📝 Proposed fix for the documentation 
-// BenchmarkKmsSignerSign test kms round trip
-// export EVNODE_E2E_GCP_KMS_KEY_NAME=projects/<project-id>/locations/<region>/keyRings/<keyring-name>/cryptoKeys/<key-name>/cryptoKeyVersions/1
-// export EVNODE_E2E_GCP_KMS_CREDENTIALS_FILE=...
-// go test -v -bench=BenchmarkKmsSignerSign -benchtime=3s -count=10 -run='^$' ./pkg/signer/gcp
+// BenchmarkKmsSignerSign test kms round trip
+// export EVNODE_E2E_AWS_KMS_KEY_ID="arn:aws:kms:...."
+// export EVNODE_E2E_AWS_KMS_REGION=eu-west-1
+// go test -v -bench=BenchmarkKmsSignerSign -benchtime=3s -count=10 -run='^$' ./pkg/signer/aws
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/signer/aws/signer_benchmark_test.go` around lines 16 - 18, Update the
benchmark comment in signer_benchmark_test.go to use the correct AWS environment
variables and test path: replace EVNODE_E2E_GCP_KMS_KEY_NAME and
EVNODE_E2E_GCP_KMS_CREDENTIALS_FILE with the AWS equivalents (e.g.,
EVNODE_E2E_AWS_KMS_KEY_ID and an AWS credentials/secret var) and include
EVNODE_E2E_AWS_KMS_REGION (or the region vars used elsewhere in this file), and
change the go test path from ./pkg/signer/gcp to ./pkg/signer/aws so the
documented commands match the AWS KMS benchmark in this file.

func BenchmarkKmsSignerSign(b *testing.B) {
keyID := os.Getenv("EVNODE_E2E_AWS_KMS_KEY_ID")
if keyID == "" {
Expand Down
2 changes: 1 addition & 1 deletion pkg/signer/gcp/signer_benchmark_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
// BenchmarkKmsSignerSign test kms round trip
// export EVNODE_E2E_AWS_KMS_KEY_ID="arn:aws:kms:...."
// export EVNODE_E2E_AWS_KMS_REGION=eu-west-1
// go test -v -bench=BenchmarkKmsSignerSign -benchtime=3s -count=10 -run='^$' ./pkg/signer/aws
// go test -v -bench=BenchmarkKmsSignerSign -benchtime=3s -count=10 -run='^$' ./pkg/signer/aws
Comment on lines 16 to +18
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Wrong environment variables and test path in benchmark documentation.

The comment documentation has critical errors:

  1. Lines 16-17 reference AWS KMS environment variables (EVNODE_E2E_AWS_KMS_KEY_ID, EVNODE_E2E_AWS_KMS_REGION), but this is a GCP KMS benchmark test that actually uses EVNODE_E2E_GCP_KMS_KEY_NAME (line 20) and EVNODE_E2E_GCP_KMS_CREDENTIALS_FILE (line 25).

  2. Line 18 references the wrong test path ./pkg/signer/aws, but this file is pkg/signer/gcp/signer_benchmark_test.go, so the path should be ./pkg/signer/gcp.

Developers following these comments will set the wrong environment variables and target the wrong package.

📝 Proposed fix for the documentation 
-// BenchmarkKmsSignerSign test kms round trip
-// export EVNODE_E2E_AWS_KMS_KEY_ID="arn:aws:kms:...."
-// export EVNODE_E2E_AWS_KMS_REGION=eu-west-1
-// go test -v -bench=BenchmarkKmsSignerSign -benchtime=3s -count=10 -run='^$' ./pkg/signer/aws
+// BenchmarkKmsSignerSign test kms round trip
+// export EVNODE_E2E_GCP_KMS_KEY_NAME=projects/<project-id>/locations/<region>/keyRings/<keyring-name>/cryptoKeys/<key-name>/cryptoKeyVersions/1
+// export EVNODE_E2E_GCP_KMS_CREDENTIALS_FILE=...
+// go test -v -bench=BenchmarkKmsSignerSign -benchtime=3s -count=10 -run='^$' ./pkg/signer/gcp
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/signer/gcp/signer_benchmark_test.go` around lines 16 - 18, Update the
benchmark comment in pkg/signer/gcp/signer_benchmark_test.go to reference the
correct GCP env vars and package path: replace the AWS env vars
(EVNODE_E2E_AWS_KMS_KEY_ID, EVNODE_E2E_AWS_KMS_REGION) with the GCP ones used by
this benchmark (EVNODE_E2E_GCP_KMS_KEY_NAME and
EVNODE_E2E_GCP_KMS_CREDENTIALS_FILE) and change the test path from
./pkg/signer/aws to ./pkg/signer/gcp (the comment pertains to
BenchmarkKmsSignerSign in this GCP signer test).

func BenchmarkKmsSignerSign(b *testing.B) {
keyName := os.Getenv("EVNODE_E2E_GCP_KMS_KEY_NAME")
if keyName == "" {
Expand Down