Skip to content

Conversation

@ilyaluk
Copy link
Contributor

@ilyaluk ilyaluk commented Jun 20, 2025

📝 Summary

Engine API auth includes optional "id" claim which CL might set to its, well, id. This change adds logging not only remote IP of the CL node, but also its "id", if set.

⛱ Motivation and Context

It's hard to debug issues if only IP is seen in logs, and you need to map back from IP to exact CL node this IP refers to. If CL node sets its id in tokens, log it. This improves troubleshooting.

📚 References

https://github.com/ethereum/execution-apis/blob/main/src/engine/authentication.md#jwt-claims


✅ I have run these commands

  • make lint
  • make test-race
  • go mod tidy
  • I have seen and agree to CONTRIBUTING.md

@ilyaluk ilyaluk requested review from Copilot and metachris June 20, 2025 19:27

This comment was marked as outdated.

@ilyaluk ilyaluk force-pushed the ilya/DVO-1024-log-cl-id-claim branch from 68495f6 to 50c6f9f Compare June 20, 2025 19:30
Engine API auth includes optional "id" claim which CL might set to its,
well, id. To improve troubleshooting, log not only remote IP of the CL
node, but also its "id", if set.
@ilyaluk ilyaluk force-pushed the ilya/DVO-1024-log-cl-id-claim branch from 50c6f9f to 6dbe25e Compare June 20, 2025 19:33
@ilyaluk ilyaluk requested a review from Copilot June 20, 2025 19:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR logs the "id" claim from a CL authorization token to enhance traceability when debugging Engine API authentication issues.

  • Extracts and logs the optional "id" claim from JWT tokens in proxy.go.
  • Updates function signatures to pass a logger for enhanced logging context.
  • Migrates the JWT dependency to v5 and updates token generation accordingly.

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
proxy.go Adds functions to extract the "id" claim and updates logging usage.
go.mod Updates the JWT dependency to "github.com/golang-jwt/jwt/v5".
cmd/jwt-tokens-service/main.go Updates JWT token generation to match the new dependency.
Comments suppressed due to low confidence (1)

cmd/jwt-tokens-service/main.go:68

  • [nitpick] Confirm that using time.Now() instead of jwt.TimeFunc() for the 'iat' claim is intentional, as it might affect test scenarios that rely on the overridable time function.
	claims["iat"] = time.Now().Unix()

@ilyaluk ilyaluk requested a review from TymKh June 24, 2025 16:48
@ilyaluk ilyaluk merged commit dedc833 into main Jul 7, 2025
2 checks passed
@ilyaluk ilyaluk deleted the ilya/DVO-1024-log-cl-id-claim branch July 7, 2025 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants