Add EVM Signer tests and implement Solana, Sui, and Tron facilitators#36
Add EVM Signer tests and implement Solana, Sui, and Tron facilitators#36
Conversation
gosunuts
commented
Feb 1, 2026
gosunuts
commented
- Created unit tests for EVMSigner including validation, address retrieval, callback signing, EIP-712 verification, and encoding.
- Implemented Solana facilitator with methods for verifying and settling payments.
- Added Sui and Tron facilitators with basic structure and unimplemented methods.
- Introduced types for Solana payloads.
- Added x402 SDK types and functions for better integration with the x402 framework.
- Created unit tests for EVMSigner including validation, address retrieval, callback signing, EIP-712 verification, and encoding. - Implemented Solana facilitator with methods for verifying and settling payments. - Added Sui and Tron facilitators with basic structure and unimplemented methods. - Introduced types for Solana payloads. - Added x402 SDK types and functions for better integration with the x402 framework.
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR executes a significant architectural refactoring to integrate the x402 SDK as the primary payment processing engine. Legacy EVM/Solana/Sui/Tron implementations are removed in favor of thin SDK wrappers. The facilitator interface shifts to use the SDK's SchemeNetworkFacilitator type. Configuration loading is refactored to support multi-source priority (CLI flags > environment variables > config file > defaults). API payloads are bumped from V1 to V2. Changes
Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
api/client/client.go (1)
41-61:⚠️ Potential issue | 🟠 MajorAlign PaymentHeader.X402Version with request version.
Only the top-levelX402Versionis set; the embedded payload can still be V0/V1, which risks SDK-side version checks failing or behaving inconsistently. Set the payload version before sending.✅ Suggested fix
func (c *Client) Verify(ctx context.Context, payload *types.PaymentPayload, req *types.PaymentRequirements) (*types.PaymentVerifyResponse, error) { - body := types.PaymentVerifyRequest{ + payloadCopy := *payload + payloadCopy.X402Version = int(types.X402VersionV2) + body := types.PaymentVerifyRequest{ X402Version: int(types.X402VersionV2), - PaymentHeader: *payload, + PaymentHeader: payloadCopy, PaymentRequirements: *req, } @@ func (c *Client) Settle(ctx context.Context, payload *types.PaymentPayload, req *types.PaymentRequirements) (*types.PaymentSettleResponse, error) { - body := types.PaymentSettleRequest{ + payloadCopy := *payload + payloadCopy.X402Version = int(types.X402VersionV2) + body := types.PaymentSettleRequest{ X402Version: int(types.X402VersionV2), - PaymentHeader: *payload, + PaymentHeader: payloadCopy, PaymentRequirements: *req, }api/server.go (2)
60-85:⚠️ Potential issue | 🟠 MajorValidate x402Version before calling facilitator.
Both handlers ignore the version fields, so unsupported or mismatched versions slip through. Validate the request version and sync/verify the payload header before invoking the SDK.✅ Suggested fix
settleRequest := &types.PaymentSettleRequest{} if err := json.NewDecoder(c.Request().Body).Decode(settleRequest); err != nil { return echo.NewHTTPError(http.StatusBadRequest, "Received malformed settlement request") } + if settleRequest.X402Version != int(types.X402VersionV2) { + return echo.NewHTTPError(http.StatusBadRequest, "unsupported x402Version") + } + if settleRequest.PaymentHeader.X402Version != 0 && settleRequest.PaymentHeader.X402Version != settleRequest.X402Version { + return echo.NewHTTPError(http.StatusBadRequest, "mismatched x402Version") + } + if settleRequest.PaymentHeader.X402Version == 0 { + settleRequest.PaymentHeader.X402Version = settleRequest.X402Version + } @@ requirement := &types.PaymentVerifyRequest{} if err := json.NewDecoder(c.Request().Body).Decode(requirement); err != nil { return echo.NewHTTPError(http.StatusBadRequest, "Received malformed payment requirements") } + if requirement.X402Version != int(types.X402VersionV2) { + return echo.NewHTTPError(http.StatusBadRequest, "unsupported x402Version") + } + if requirement.PaymentHeader.X402Version != 0 && requirement.PaymentHeader.X402Version != requirement.X402Version { + return echo.NewHTTPError(http.StatusBadRequest, "mismatched x402Version") + } + if requirement.PaymentHeader.X402Version == 0 { + requirement.PaymentHeader.X402Version = requirement.X402Version + }Also applies to: 99-125
20-31:⚠️ Potential issue | 🟠 Major/supported hardcodes an EVM network.
Returningeip155:*is incorrect for Solana/Sui/Tron and misleads clients. Plumb the configured network into the server (or have the facilitator expose it) and use that here.✅ Suggested fix (requires updating call sites)
type server struct { *echo.Echo facilitator types.SchemeNetworkFacilitator + network string } @@ -func NewServer(facilitator types.SchemeNetworkFacilitator) *server { +func NewServer(facilitator types.SchemeNetworkFacilitator, network string) *server { s := &server{ Echo: echo.New(), facilitator: facilitator, + network: network, } @@ kinds := []*types.SupportedKind{ { Scheme: s.facilitator.Scheme(), - Network: "eip155:*", // TODO: Get actual network from config + Network: s.network, }, }Also applies to: 136-141
🤖 Fix all issues with AI agents
In `@cmd/client/config.go`:
- Around line 68-74: The env.Provider callback currently returns the raw env
name, so keys like X402_CLIENT_URL don't match mapstructure tags; update the
callback passed to k.Load(env.Provider(...)) in cmd/client/config.go (prefix
"X402_CLIENT_") and the analogous call in cmd/facilitator/config.go (prefix
"X402_") to strip the prefix and normalize the remaining key (e.g.,
strings.TrimPrefix(s, "X402_CLIENT_") then strings.ToLower(...)); if your config
uses nested keys convert remaining underscores to koanf dots (replace "_" with
".") before lowercasing so environment names map correctly to your mapstructure
tags.
In `@cmd/facilitator/config.go`:
- Around line 62-67: The env.Provider call used when loading environment
variables passes an identity transform that leaves keys uppercase so koanf
cannot match lowercase defaults; update the transform passed into
k.Load/env.Provider to normalize keys to lowercase (e.g., replace the current
func(s string) string { return s } with a function that returns
strings.ToLower(s)) so environment vars like X402_SCHEME will map to the
lowercase koanf keys; apply the same change to the corresponding env.Provider
invocation used in the client config.
In `@facilitator/evm/facilitator.go`:
- Around line 142-146: GetSigners currently returns an empty slice; wire it to
return the real signer addresses from the facilitator state. In GetSigners,
query the underlying signer attached to the Facilitator (e.g., f.signer or
f.Signer) for its address list (call the signer method that exposes addresses
such as Addresses() or Address()/PublicKey() and map to hex strings), and if the
facilitator stores configured signers in config (e.g., f.cfg or f.config with a
Signers/SignerAddresses field) use that as a fallback; return the
combined/validated []string of addresses instead of []string{}.
In `@facilitator/evm/signer/evm_signer.go`:
- Around line 59-118: NewEVMSigner leaks the RPC client on several early-return
error paths and allows a callback signer with no addresses; ensure the
ethclient.Client is closed on every error after successful ethclient.Dial by
calling client.Close() before each fmt.Errorf return (references: NewEVMSigner,
client), and enforce that when config.Signer != nil the config.Addresses (or
signer.addresses) is non-empty (return an error and close client if empty) since
callbacks cannot derive addresses; keep the existing private-key flow
(createPrivateKeySigner, privateKey -> signer.addresses) unchanged but add
client.Close() on its failure paths as well.
- Around line 405-423: The call to bind.WaitMined is using an empty
ethTypes.Transaction so it never waits for the real tx; instead, first fetch the
transaction by hash (use the existing hash variable created from txHash and
s.client.TransactionByHash or s.client.TransactionReceipt lookup) to obtain the
*ethTypes.Transaction, then pass that transaction to bind.WaitMined(ctx,
s.client, tx) before calling s.client.TransactionReceipt(ctx, hash); update the
flow in evm_signer.go around the hash variable, bind.WaitMined, and
TransactionReceipt calls so WaitMined receives the actual transaction object.
- Around line 267-335: The VerifyTypedData flow currently builds
TypedData/TypedDataDomain and calls HashTypedData, but the encoder
(HashTypedData and the underlying domain/type encoder) mis-implements EIP-712 by
appending verifyingContract as raw 20 bytes and not handling arrays, bytesN, or
nested types; to fix, replace the custom encoder with go-ethereum's canonical
implementation (github.com/ethereum/go-ethereum/signer/core/apitypes) when
computing the domain and message hashes inside HashTypedData (or fully refactor
HashTypedData to follow apitypes’ EncodeType/HashStruct logic), and if you must
keep the custom encoder, ensure verifyingContract is encoded as a 32-byte
left-padded value and extend type serialization to generically support arrays,
fixed-size bytes (bytesN), and nested/dependent types per EIP-712 spec so
VerifyTypedData recovers the correct digest and address.
In `@facilitator/solana/facilitator.go`:
- Around line 21-22: The local variable in NewFacilitator shadows the imported
client package; rename the local variable (e.g., from client to solClient or
remoteClient) when assigning client.NewClient(url) and update all subsequent
references inside NewFacilitator (including the later use at the line currently
flagged) to that new name so the imported package remains accessible.
- Around line 21-39: NewFacilitator currently accepts a network string but
doesn't use or store it; update the Facilitator implementation to follow the EVM
pattern by validating the network value (e.g., allowed Solana networks like
"mainnet", "testnet", "devnet" or your project's canonical set) and add a
network field to the Facilitator struct, then assign the validated value in
NewFacilitator; ensure downstream methods that reference network (GetExtra,
GetSigners) read the stored Facilitator.network. Alternatively, if you prefer to
drop the parameter, remove the network argument from NewFacilitator and all
callers, but do not leave an unused parameter; update NewFacilitator, the
Facilitator struct, and any callers accordingly.
In `@facilitator/sui/facilitator.go`:
- Around line 12-17: The Facilitator constructor currently drops all inputs and
the struct is empty; update the Facilitator type to include fields for network,
url, and privateKeyHex (or parsed/validated equivalents like
scheme/client/privateKey) and modify NewFacilitator to validate parameters
(non-empty network, valid URL format, and non-empty/hex-valid privateKeyHex)
before returning the instance; ensure NewFacilitator stores the values into the
Facilitator fields (and if applicable, parse/validate the private key similarly
to the Solana/EVM facilitators and return an error on invalid input) so callers
fail fast and the struct retains the provided configuration.
In `@facilitator/tron/facilitator.go`:
- Around line 15-25: NewFacilitator currently returns a usable Facilitator while
Verify and Settle always error, allowing the server to start but fail at
runtime; change NewFacilitator to fail-fast by returning an error until the Tron
implementation is complete (or alternatively fully construct and store required
config on the Facilitator). Specifically, update NewFacilitator to validate
inputs (network, url, privateKeyHex) and return a descriptive error (e.g., "tron
facilitator not implemented" or input validation errors) instead of
&Facilitator{}, so callers cannot obtain a non-functional Facilitator instance;
keep Verify and Settle unchanged until implemented or implement them together
with proper initialization.
🧹 Nitpick comments (5)
facilitator/sui/facilitator.go (1)
42-45:GetSignersreturns empty slice but should return the facilitator's address.Once you store the private key and derive the address,
GetSignersshould return it. The EVM facilitator derives the signer address from the private key. Right now this will return[]string{}which may cause confusion when querying supported kinds.Add a
// TODOor leave a comment indicating this needs implementation when the struct fields are populated.facilitator/solana/facilitator.go (1)
59-69: Unusednetworkparameter inGetExtraandGetSigners.Both methods accept
network types.Networkbut ignore it. If the network is irrelevant for Solana (single fee payer across networks), consider documenting this or using_to signal intentional discard:-func (t *Facilitator) GetExtra(network types.Network) map[string]interface{} { +func (t *Facilitator) GetExtra(_ types.Network) map[string]interface{} {-func (t *Facilitator) GetSigners(network types.Network) []string { +func (t *Facilitator) GetSigners(_ types.Network) []string {facilitator/evm/facilitator_test.go (1)
31-40: Avoid commented-out tests; keep them skipped or build-tagged.Commented code rots. Prefer real tests with
t.Skip(or build tags) so they stay compiled and discoverable.🔧 Example rewrite
-// func TestEVMFacilitatorVerifyIntegration(t *testing.T) { -// t.Skip("Integration test - requires actual RPC connection") -// // TODO: Implement SDK-based integration test -// } +func TestEVMFacilitatorVerifyIntegration(t *testing.T) { + t.Skip("Integration test - requires actual RPC connection") + // TODO: Implement SDK-based integration test +} // -// func TestEVMFacilitatorSettleIntegration(t *testing.T) { -// t.Skip("Integration test - requires actual RPC connection") -// // TODO: Implement SDK-based integration test -// } +func TestEVMFacilitatorSettleIntegration(t *testing.T) { + t.Skip("Integration test - requires actual RPC connection") + // TODO: Implement SDK-based integration test +}cmd/client/main.go (2)
21-29: Avoid string-matching help errors; useerrors.Is.Rely on
pflag.ErrHelpto prevent fragile string comparisons.🔧 Suggested change
import ( + "errors" "fmt" "os" @@ + "github.com/spf13/pflag" @@ - if err.Error() == "flag: help requested" { + if errors.Is(err, pflag.ErrHelp) { printUsage() os.Exit(0) }
40-60: TODO leaves the main path as a no‑op.If this is intended as a stub, consider gating it behind a clear message or implementing the SDK flow now. I can help wire up
ClientEvmSignerand payload creation if you want.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (34)
api/client/client.goapi/server.gocmd/client/config.gocmd/client/main.gocmd/facilitator/config.gocmd/facilitator/main.gofacilitator/evm.gofacilitator/evm/facilitator.gofacilitator/evm/facilitator_test.gofacilitator/evm/signer/evm_signer.gofacilitator/evm/signer/evm_signer_test.gofacilitator/evm_test.gofacilitator/iface.gofacilitator/solana.gofacilitator/solana/facilitator.gofacilitator/solana/types.gofacilitator/sui.gofacilitator/sui/facilitator.gofacilitator/sui/sui.gofacilitator/tron.gofacilitator/tron/facilitator.gofacilitator/tron/tron.gogo.modinternal/sdk/sdk_test.goscheme/evm/chaincfg.goscheme/evm/crypto.goscheme/evm/eip3009/eip3009.abischeme/evm/eip3009/eip3009.goscheme/evm/signer.goscheme/evm/signer_test.goscheme/evm/types.gotypes/facilitator.gotypes/scheme.gotypes/x402.go
💤 Files with no reviewable changes (13)
- facilitator/solana.go
- facilitator/tron.go
- scheme/evm/signer.go
- scheme/evm/crypto.go
- internal/sdk/sdk_test.go
- scheme/evm/eip3009/eip3009.abi
- scheme/evm/signer_test.go
- facilitator/evm_test.go
- facilitator/evm.go
- scheme/evm/types.go
- scheme/evm/eip3009/eip3009.go
- scheme/evm/chaincfg.go
- facilitator/sui.go
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-27T12:45:06.366Z
Learnt from: CR
Repo: gosuda/portal PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-27T12:45:06.366Z
Learning: Applies to **/*.go : Import order must follow: stdlib -> external -> internal (blank-line separated)
Applied to files:
go.mod
📚 Learning: 2026-01-27T12:45:06.366Z
Learnt from: CR
Repo: gosuda/portal PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-27T12:45:06.366Z
Learning: Applies to **/*.go : Avoid `math/rand` for security-sensitive operations in Go
Applied to files:
go.mod
🧬 Code graph analysis (11)
api/client/client.go (1)
types/facilitator.go (2)
X402Version(102-102)X402VersionV2(106-106)
facilitator/sui/facilitator.go (5)
facilitator/evm/facilitator.go (2)
NewFacilitator(22-61)Facilitator(16-19)facilitator/iface.go (1)
NewFacilitator(15-28)facilitator/solana/facilitator.go (2)
NewFacilitator(21-39)Facilitator(15-19)types/facilitator.go (2)
PaymentPayload(15-17)PaymentRequirements(25-39)types/scheme.go (2)
Scheme(4-4)Sui(9-9)
types/x402.go (1)
types/facilitator.go (3)
PaymentPayload(15-17)PaymentRequirements(25-39)SupportedKind(91-94)
facilitator/evm/signer/evm_signer_test.go (3)
facilitator/evm/signer/evm_signer.go (8)
EVMSignerConfig(41-55)NewEVMSigner(59-121)TypedDataDomain(494-499)Type(487-490)TypedData(501-506)Types(492-492)HashTypedData(509-525)EVMSigner(33-38)types/scheme.go (1)
Signer(14-14)types/x402.go (3)
TypedDataDomain(54-54)TypedDataField(57-57)FacilitatorEvmSigner(48-48)
facilitator/evm/facilitator_test.go (2)
facilitator/evm/facilitator.go (1)
NewFacilitator(22-61)facilitator/iface.go (1)
NewFacilitator(15-28)
cmd/facilitator/config.go (1)
cmd/client/config.go (4)
LoadConfig(32-88)Config(16-25)GetEnvOrDefault(126-131)GetEnvIntOrDefault(134-141)
cmd/client/config.go (3)
types/scheme.go (1)
Scheme(4-4)types/x402.go (1)
Network(24-24)cmd/facilitator/config.go (2)
LoadConfig(30-82)Config(17-23)
cmd/client/main.go (4)
cmd/client/config.go (1)
LoadConfig(32-88)types/scheme.go (1)
Scheme(4-4)types/x402.go (1)
Network(24-24)api/client/client.go (1)
NewClient(21-30)
facilitator/solana/facilitator.go (4)
types/scheme.go (2)
Scheme(4-4)Solana(8-8)facilitator/iface.go (1)
NewFacilitator(15-28)types/x402.go (4)
NewFacilitator(84-84)VerifyResponse(27-27)SettleResponse(30-30)Network(24-24)types/facilitator.go (2)
PaymentPayload(15-17)PaymentRequirements(25-39)
types/facilitator.go (2)
types/scheme.go (1)
Scheme(4-4)types/x402.go (1)
Network(24-24)
facilitator/evm/signer/evm_signer.go (2)
types/scheme.go (1)
Signer(14-14)types/x402.go (4)
TypedDataDomain(54-54)TypedDataField(57-57)TransactionReceipt(60-60)FacilitatorEvmSigner(48-48)
🔍 Remote MCP Context7
Now let me search for more information about facilitator pattern implementations and Solana/Sui/Tron payment verification:Perfect! I've gathered sufficient context. Let me compile the findings into a comprehensive summary for the PR review.
Summary of Additional Context for PR Review
Based on my search for relevant documentation and patterns, here are the key insights to support your PR review:
1. x402 SDK Integration & SchemeNetworkFacilitator Interface
The x402 SDK defines a SchemeNetworkFacilitator interface that includes methods verify() and settle() for payment verification and settlement. Facilitators are abstracted services that verify and settle transactions, abstracting blockchain complexity from API servers. The PR's migration from the internal facilitator.Facilitator interface to the SDK's SchemeNetworkFacilitator aligns with this architecture pattern.
2. EIP-712 Implementation Correctness
EIP-712 signing computes a signature using keccak256("\x19\x01" ‖ domainSeparator ‖ hashStruct(message)). The EVMSigner's VerifyTypedData method in the PR implements this hashing approach. EIP-712 is a standard for hashing and signing typed structured data, allowing users to sign human-readable messages rather than opaque hashes. The implementation provides both signing and recovery mechanisms critical for payment verification.
3. Koanf Configuration Management Best Practices
Configuration from multiple sources can be loaded and merged into a koanf instance with provider and parser interfaces enabling configuration from multiple sources and formats. The PR's config files implement the correct pattern: The bundled posflag.Provider takes a flagset from spf13/pflag, and passing the Koanf instance to posflag helps it deal with default command line flag values. This replaces the previous Cobra CLI approach appropriately.
4. ECDSA Signature Recovery & Verification
Ecrecover (elliptic curve signature recover) can be called from the go-ethereum crypto package to retrieve the public key of the signer from hash and signature bytes. The Decred secp256k1 library's RecoverCompact function converts signatures to secp256k1 format with recovery ID handling, which the EVMSigner uses for signature verification. This is relevant for validating EVM-based payment signatures.
5. Solana Payment Verification Context
x402 relies on Facilitators to verify and settle transactions so that servers don't need to directly interact with blockchain infrastructure, abstracting blockchain payments on behalf of API servers. Each Signature in Solana is 64 bytes and is created by signing the transaction's Message with the account's private key, with a signature required for each signer account. The new Solana facilitator skeleton correctly positions itself to implement these signing patterns.
6. Key Changes Alignment
- API Type Updates (
types/facilitator.go): The addition ofPaymentPayload,PaymentRequirementswrappers withX402VersionV2constant properly extends the local types while embedding SDK types. - Configuration Loading Migration: Switching from Cobra to Koanf with priority ordering (CLI > env > file > defaults) is appropriate for multi-environment deployments.
- Facilitator Architecture: Moving from concrete EVM/Solana/Sui/Tron implementations to SDK-based wrappers reduces duplication and improves maintainability., [::web_search::],
🔇 Additional comments (47)
facilitator/sui/facilitator.go (1)
19-25: Stub methods returning errors—acceptable for now.Returning explicit "not implemented" errors is fine for a placeholder. However, consider using a sentinel error type or constant so callers can programmatically detect unimplemented functionality rather than string-matching.
facilitator/solana/facilitator.go (2)
41-47: Stub implementations return errors - acceptable for scaffolding.Both
VerifyandSettlereturn "not implemented" errors. This aligns with the PR description stating these are placeholders. Ensure follow-up work is tracked.
49-57: Accessors look correct.
Scheme()andCaipFamily()return expected values consistent with the Solana scheme definition.types/x402.go (13)
1-11: Clean import structure.Import aliases (
x402,x402types) prevent naming collisions and make the re-export intent clear.
17-31: Solid aliasing strategy.The
SDKprefix clearly distinguishes raw SDK types from the extended wrapper types intypes/facilitator.go. Consumers can choose between direct SDK types for interop or wrapper types with additional fields.
37-64: EVM type surface is comprehensive.Covers the full payment flow: authorization structs, payload types, both signer interfaces, and transaction receipts.
70-76: Facilitator types correctly exposed.
SchemeNetworkFacilitatoris the core abstraction for V2 payment verification and settlement. Proper foundation for the new architecture.
82-88: Constructor re-exports are correct.Note: The SDK uses
Newx402Facilitator(lowercase 'x'), which the alias faithfully preserves.
94-103: Full signature verification surface exposed.Covers EOA, smart contract wallets (EIP-1271), and counterfactual signatures (ERC-6492).
109-115: EIP-712 hashing utilities properly exposed.Both EIP-3009 and Permit2 authorization hashing available.
121-127: ERC-6492 support included.Enables counterfactual signature validation for smart contract wallets that haven't been deployed yet.
133-145: Payload detection and conversion utilities complete.Runtime type discrimination and map-based deserialization properly exposed.
151-160: Network configuration functions properly exposed.
GetEvmChainIdhandles CAIP-2 parsing; others provide dynamic config lookup.
166-178: Essential utilities exposed.
CreateNonceis critical for replay attack prevention in EIP-3009. Amount parsing/formatting handles decimal-to-smallest-unit conversions.
184-193: Constants correctly exposed.Canonical addresses and status codes available for payment processing.
199-211: ABI re-exports complete.Both signature formats (VRS, bytes) for TransferWithAuthorization available, plus ERC20 and authorization state ABIs.
go.mod (3)
16-16: LGTM: pflag addition aligns with Koanf migration.Replacing Cobra with pflag for CLI flag parsing is the correct approach when using Koanf's posflag provider.
35-35: Dependency classification is correct.No direct imports of
github.com/decred/dcrd/dcrec/secp256k1/v4exist in the codebase. The// indirectmarking is justified—any usage flows through transitive dependencies, not direct code.Likely an incorrect or invalid review comment.
10-12:⚠️ Potential issue | 🟡 MinorUpdate Koanf provider dependencies to latest stable versions.
The Koanf providers have newer stable releases available:
koanf/providers/envis at v1.0.0, but v1.1.0 is availablekoanf/providers/fileis at v1.2.0, but v1.2.1 is availablekoanf/providers/posflagis current at v1.0.1Update these to their latest versions unless pinned intentionally.
⛔ Skipped due to learnings
Learnt from: CR Repo: gosuda/portal PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-27T12:45:06.366Z Learning: Applies to go.mod : Pin Go toolchain version to match `go.mod` (currently 1.25.3)Learnt from: CR Repo: gosuda/portal PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-27T12:45:06.366Z Learning: Use `govulncheck` and `go mod verify` during release workflowsfacilitator/evm/facilitator_test.go (1)
10-29: Constructor validation coverage is solid.Covers the required inputs cleanly with readable subtests.
types/scheme.go (1)
3-14: Signer callback type is clean and minimal.Good public surface for signer injection.
facilitator/iface.go (1)
13-27: Scheme routing looks straightforward.Clean delegation to per-scheme facilitators.
cmd/client/config.go (3)
15-25: Config mapping is clear.Field tags align with loader keys.
90-123: Usage text matches flags and priority.Clear and consistent with the loader.
125-141: Env helper utilities are straightforward.No concerns here.
cmd/facilitator/config.go (3)
16-23: Config struct is clear and focused.No issues.
84-111: Usage text is aligned with flags and defaults.Looks consistent.
113-129: Env helpers are straightforward.No concerns.
facilitator/evm/facilitator.go (6)
21-61: Constructor validation and cleanup look solid.Good input checks and cleanup on scheme creation failure.
63-100: Callback-based constructor is a good extension point.Nice support for external key management.
102-124: SDK delegation for verify/settle is clean.Error wrapping is appropriate.
126-134: Confirm scheme identifiers match the registry.Please verify that
"exact"and"eip155:*"are the expected identifiers for the supported‑kinds response.
136-140:GetExtrareturning nil is fine for EVM.No extra metadata needed.
148-150:GetSchemepassthrough is fine.No issues.
types/facilitator.go (2)
13-44: Wrapper types + SDK conversion look solid.
Clean separation between SDK payloads and server metadata.
101-112: Version helper is concise and clear.cmd/facilitator/main.go (1)
18-77: Startup + graceful shutdown wiring looks solid.facilitator/evm/signer/evm_signer_test.go (5)
16-121: Config validation + callback precedence tests are solid.
123-257: Typed-data verification and hashing coverage looks good.
259-323: EncodeValue test matrix is well-rounded.
325-357: Interface compliance and address config tests look fine.
359-445: Helper, integration placeholder, and benchmarks are fine as scaffolding.facilitator/evm/signer/evm_signer.go (5)
123-141: Signer helper + address accessor look good.
143-265: Contract read/write flow is straightforward.
337-398: Raw transaction send path looks consistent.
425-483: Balance/code helpers and Close are straightforward.
641-655: Utility helpers and interface assertion are fine.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // Load from environment variables (X402_CLIENT_*) | ||
| // Example: X402_CLIENT_URL=http://localhost:9090 X402_CLIENT_FROM=0x... | ||
| if err := k.Load(env.Provider("X402_CLIENT_", ".", func(s string) string { | ||
| return s | ||
| }), nil); err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the actual code in cmd/client/config.go
cat -n cmd/client/config.go | head -100Repository: gosuda/x402-facilitator
Length of output: 3765
🏁 Script executed:
# Search for imports and understand what koanf is being used
rg -A 5 "import" cmd/client/config.go | head -20Repository: gosuda/x402-facilitator
Length of output: 172
🏁 Script executed:
# Look for how config keys are accessed throughout the codebase to understand case sensitivity
rg "k\.Get|k\.Unmarshal|GetString|GetInt" --type go -A 2 | head -40Repository: gosuda/x402-facilitator
Length of output: 690
🏁 Script executed:
# Check for any other env.Provider usages that might show the pattern
rg "env\.Provider" --type go -B 2 -A 5Repository: gosuda/x402-facilitator
Length of output: 930
🏁 Script executed:
# Look for any tests related to config loading
find . -name "*test*.go" -type f | grep -E "(config|client)" | head -10Repository: gosuda/x402-facilitator
Length of output: 49
🏁 Script executed:
# Check if there's documentation about koanf usage
rg -l "koanf" --type go | head -10Repository: gosuda/x402-facilitator
Length of output: 156
🌐 Web query:
koanf env.Provider case sensitivity normalization
💡 Result:
- koanf keys are case-sensitive, so
app.server.portandAPP.SERVER.portare different keys unless you normalize them yourself. [1] env.Provider(prefix, delim, cb)does not automatically normalize case; instead it gives you a callbackcbto transform env var names (e.g., lowercase, strip prefix, replace_with.). Ifcbreturns"", that env var is ignored. [2]- The
prefixfilter is case-sensitive. [2] - Typical normalization pattern (lowercase + strip prefix +
_→.) is shown in the official docs/examples. [3]
Example normalization callback (from docs pattern):
k.Load(env.Provider("MYVAR_", ".", func(s string) string {
return strings.Replace(
strings.ToLower(strings.TrimPrefix(s, "MYVAR_")),
"_", ".", -1,
)
}), nil)[2][3]
Callback must normalize keys—ToLower alone is insufficient.
koanf keys are case-sensitive, and the current identity transform returns X402_CLIENT_URL as-is. When unmarshaling to mapstructure tags like "url", the key won't match even with case-insensitivity because the full prefixed name (x402_client_url) differs from the tag (url).
The callback needs to both strip the prefix and normalize to lowercase per koanf's documented pattern:
🔧 Correct normalization
import (
"os"
"strconv"
+ "strings"
@@
- if err := k.Load(env.Provider("X402_CLIENT_", ".", func(s string) string {
- return s
+ if err := k.Load(env.Provider("X402_CLIENT_", ".", func(s string) string {
+ return strings.ToLower(strings.TrimPrefix(s, "X402_CLIENT_"))
}), nil); err != nil {
return nil, err
}Same issue in cmd/facilitator/config.go with prefix "X402_".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Load from environment variables (X402_CLIENT_*) | |
| // Example: X402_CLIENT_URL=http://localhost:9090 X402_CLIENT_FROM=0x... | |
| if err := k.Load(env.Provider("X402_CLIENT_", ".", func(s string) string { | |
| return s | |
| }), nil); err != nil { | |
| return nil, err | |
| } | |
| import ( | |
| "os" | |
| "strconv" | |
| "strings" | |
| ) | |
| // ... other code ... | |
| // Load from environment variables (X402_CLIENT_*) | |
| // Example: X402_CLIENT_URL=http://localhost:9090 X402_CLIENT_FROM=0x... | |
| if err := k.Load(env.Provider("X402_CLIENT_", ".", func(s string) string { | |
| return strings.ToLower(strings.TrimPrefix(s, "X402_CLIENT_")) | |
| }), nil); err != nil { | |
| return nil, err | |
| } |
🤖 Prompt for AI Agents
In `@cmd/client/config.go` around lines 68 - 74, The env.Provider callback
currently returns the raw env name, so keys like X402_CLIENT_URL don't match
mapstructure tags; update the callback passed to k.Load(env.Provider(...)) in
cmd/client/config.go (prefix "X402_CLIENT_") and the analogous call in
cmd/facilitator/config.go (prefix "X402_") to strip the prefix and normalize the
remaining key (e.g., strings.TrimPrefix(s, "X402_CLIENT_") then
strings.ToLower(...)); if your config uses nested keys convert remaining
underscores to koanf dots (replace "_" with ".") before lowercasing so
environment names map correctly to your mapstructure tags.
| // Load from environment variables (X402_*) | ||
| // Example: X402_SCHEME=evm X402_NETWORK=base-sepolia | ||
| if err := k.Load(env.Provider("X402_", ".", func(s string) string { | ||
| return s | ||
| }), nil); err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the config.go file and examine the context around lines 62-67
find . -name "config.go" -path "*/cmd/facilitator/*" | head -5Repository: gosuda/x402-facilitator
Length of output: 94
🏁 Script executed:
# Look at the relevant config file
cd cmd/facilitator && wc -l config.goRepository: gosuda/x402-facilitator
Length of output: 80
🏁 Script executed:
# Read the config.go file to understand the context
cat -n cmd/facilitator/config.goRepository: gosuda/x402-facilitator
Length of output: 4783
🏁 Script executed:
# Check imports and understand what env.Provider is
grep -r "env\.Provider\|from \"koanf\"\|from \"koanf/providers" cmd/facilitator/config.go -B5 -A5 | head -50Repository: gosuda/x402-facilitator
Length of output: 300
🏁 Script executed:
# Search for koanf usage and Provider documentation
fd -e go | xargs grep -l "koanf" | head -10Repository: gosuda/x402-facilitator
Length of output: 156
🏁 Script executed:
# Search for koanf usage and Provider documentation in vendor or go.mod
find . -name "go.mod" -o -name "go.sum" | xargs grep -i koanf | head -10Repository: gosuda/x402-facilitator
Length of output: 784
🏁 Script executed:
# Check if there are any tests that show how env.Provider should be used
find . -name "*test*.go" | xargs grep -l "env\.Provider" 2>/dev/nullRepository: gosuda/x402-facilitator
Length of output: 51
🏁 Script executed:
# Search for other env.Provider usage in the codebase
rg "env\.Provider" -A2 -B2Repository: gosuda/x402-facilitator
Length of output: 745
🏁 Script executed:
# Look at the cmd/client to see if it has similar config loading
cat -n cmd/client/config.go 2>/dev/null | head -100Repository: gosuda/x402-facilitator
Length of output: 3765
🌐 Web query:
koanf env.Provider key normalization case sensitivity golang
💡 Result:
- koanf keys are case-sensitive:
app.server.portandAPP.SERVER.portare different keys, and koanf itself does not auto-normalize key casing. [1] env.Providerdoes not implicitly normalize env var names; instead it lets you supply a callback to transform env var names into koanf keys (e.g., lowercase, strip prefix, replace_with.). If the callback returns"", that variable is ignored. [2]- Env prefix matching is case-sensitive: if you pass
prefix="MYAPP_", it will only capture env vars starting with exactlyMYAPP_(notmyapp_). [2] - The koanf docs/examples explicitly show lowercasing + replacing underscores in the callback, e.g. turning
MYVAR_PARENT1_CHILD1_NAMEintoparent1.child1.name. [3]
Example normalization callback:
k.Load(env.Provider("MYAPP_", ".", func(s string) string {
return strings.ReplaceAll(
strings.ToLower(strings.TrimPrefix(s, "MYAPP_")),
"_", ".",
)
}), nil)Sources: [1] [2] [3]
🌐 Web query:
koanf github readme env.Provider example normalize keys
💡 Result:
In koanf’s README, the env/v2 Provider example “normalizes” environment-variable keys by using TransformFunc to:
- lowercase the key
- strip a prefix (e.g.
MYVAR_) - replace
_with the koanf delimiter (e.g..) so nested keys work
k.Load(env.Provider(".", env.Opt{
Prefix: "MYVAR_",
TransformFunc: func(k, v string) (string, any) {
k = strings.ReplaceAll(
strings.ToLower(strings.TrimPrefix(k, "MYVAR_")),
"_", ".",
)
return k, v
},
}), nil)Example mapping: MYVAR_PARENT1_CHILD1_NAME → parent1.child1.name. [1]
If you’re using the older providers/env (non-v2) API, the equivalent idea is the callback that can “lowercase everything, strip prefixes and replace _ with .”. [2]
🏁 Script executed:
# Check go.mod for koanf version to understand which version is in use
cat go.mod | grep koanfRepository: gosuda/x402-facilitator
Length of output: 332
🏁 Script executed:
# Look for any examples or documentation in the repository about config loading
find . -name "README*" -o -name "*.md" | xargs grep -i "X402_\|environment" 2>/dev/null | head -20Repository: gosuda/x402-facilitator
Length of output: 49
🏁 Script executed:
# Check if there are any actual environment variable tests or examples showing expected behavior
find . -type f -name "*.go" | xargs grep -i "X402_SCHEME\|X402_NETWORK" 2>/dev/nullRepository: gosuda/x402-facilitator
Length of output: 149
Environment variables won't override defaults due to case mismatch.
The identity transform in the env.Provider callback leaves keys uppercase (e.g., SCHEME), while defaults and struct tags use lowercase (e.g., scheme). Since koanf keys are case-sensitive, the overrides fail silently. Normalize to lowercase:
Fix required
import (
"os"
"strconv"
+ "strings"
@@
- if err := k.Load(env.Provider("X402_", ".", func(s string) string {
- return s
+ if err := k.Load(env.Provider("X402_", ".", func(s string) string {
+ return strings.ToLower(s)
}), nil); err != nil {
return nil, err
}Same issue exists in cmd/client/config.go at the corresponding env.Provider call.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Load from environment variables (X402_*) | |
| // Example: X402_SCHEME=evm X402_NETWORK=base-sepolia | |
| if err := k.Load(env.Provider("X402_", ".", func(s string) string { | |
| return s | |
| }), nil); err != nil { | |
| return nil, err | |
| // Load from environment variables (X402_*) | |
| // Example: X402_SCHEME=evm X402_NETWORK=base-sepolia | |
| if err := k.Load(env.Provider("X402_", ".", func(s string) string { | |
| return strings.ToLower(s) | |
| }), nil); err != nil { | |
| return nil, err | |
| } |
🤖 Prompt for AI Agents
In `@cmd/facilitator/config.go` around lines 62 - 67, The env.Provider call used
when loading environment variables passes an identity transform that leaves keys
uppercase so koanf cannot match lowercase defaults; update the transform passed
into k.Load/env.Provider to normalize keys to lowercase (e.g., replace the
current func(s string) string { return s } with a function that returns
strings.ToLower(s)) so environment vars like X402_SCHEME will map to the
lowercase koanf keys; apply the same change to the corresponding env.Provider
invocation used in the client config.
| // GetSigners returns signer addresses used by this facilitator for a given network. | ||
| func (f *Facilitator) GetSigners(network types.Network) []string { | ||
| // TODO: Return actual signer addresses from the underlying signer | ||
| return []string{} | ||
| } |
There was a problem hiding this comment.
GetSigners returns empty slice; wire actual signer addresses.
This leaves the supported‑kinds response without signers. Please plumb addresses from the signer (or config) and return them here. I can help implement this if you want.
🤖 Prompt for AI Agents
In `@facilitator/evm/facilitator.go` around lines 142 - 146, GetSigners currently
returns an empty slice; wire it to return the real signer addresses from the
facilitator state. In GetSigners, query the underlying signer attached to the
Facilitator (e.g., f.signer or f.Signer) for its address list (call the signer
method that exposes addresses such as Addresses() or Address()/PublicKey() and
map to hex strings), and if the facilitator stores configured signers in config
(e.g., f.cfg or f.config with a Signers/SignerAddresses field) use that as a
fallback; return the combined/validated []string of addresses instead of
[]string{}.
| func NewEVMSigner(config *EVMSignerConfig) (*EVMSigner, error) { | ||
| if config.RpcURL == "" { | ||
| return nil, fmt.Errorf("rpc URL is required") | ||
| } | ||
|
|
||
| // Connect to RPC | ||
| client, err := ethclient.Dial(config.RpcURL) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to connect to RPC: %w", err) | ||
| } | ||
|
|
||
| // Verify connection by getting chain ID | ||
| ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) | ||
| defer cancel() | ||
|
|
||
| chainID, err := client.NetworkID(ctx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get network ID: %w", err) | ||
| } | ||
|
|
||
| // Validate chain ID matches config | ||
| if config.ChainID != 0 && chainID.Int64() != config.ChainID { | ||
| return nil, fmt.Errorf("chain ID mismatch: expected %d, got %d", config.ChainID, chainID.Int64()) | ||
| } | ||
|
|
||
| signer := &EVMSigner{ | ||
| client: client, | ||
| chainID: chainID, | ||
| } | ||
|
|
||
| // Set up addresses | ||
| if len(config.Addresses) > 0 { | ||
| for _, addr := range config.Addresses { | ||
| signer.addresses = append(signer.addresses, common.HexToAddress(addr)) | ||
| } | ||
| } | ||
|
|
||
| // Set up signing mechanism | ||
| if config.Signer != nil { | ||
| signer.signer = config.Signer | ||
| } else if config.PrivateKey != "" { | ||
| privateKey, err := crypto.HexToECDSA(strings.TrimPrefix(config.PrivateKey, "0x")) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to parse private key: %w", err) | ||
| } | ||
| signer.signer = createPrivateKeySigner(privateKey) | ||
|
|
||
| // Derive address from private key if not provided | ||
| if len(signer.addresses) == 0 { | ||
| publicKey := privateKey.Public() | ||
| publicKeyECDSA, ok := publicKey.(*ecdsa.PublicKey) | ||
| if !ok { | ||
| return nil, fmt.Errorf("failed to cast public key to ECDSA") | ||
| } | ||
| address := crypto.PubkeyToAddress(*publicKeyECDSA) | ||
| signer.addresses = append(signer.addresses, address) | ||
| } | ||
| } else { | ||
| return nil, fmt.Errorf("either PrivateKey or SignerCallback must be provided") | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "evm_signer.go" -type fRepository: gosuda/x402-facilitator
Length of output: 105
🏁 Script executed:
head -n 150 facilitator/evm/signer/evm_signer.goRepository: gosuda/x402-facilitator
Length of output: 4868
🏁 Script executed:
# Check if ethclient has Close method and when it's needed
rg "Close\(\)" facilitator/evm/signer/ -A 2 -B 2Repository: gosuda/x402-facilitator
Length of output: 714
🏁 Script executed:
# Search for patterns where EVMSigner is instantiated to understand address requirements
rg "NewEVMSigner" . -A 5 -B 2Repository: gosuda/x402-facilitator
Length of output: 3271
🏁 Script executed:
# Check the EVMSigner struct definition and Signer callback signature
rg "type EVMSigner struct|type.*Signer.*func" facilitator/evm/signer/ -A 5Repository: gosuda/x402-facilitator
Length of output: 415
Close RPC client on all error paths; validate addresses for callback signers.
The client leaks when errors occur after Dial (lines 77, 83, 104, 113). Additionally, callback-based signers can end up with an empty addresses array—the config comment claims addresses are "derived from the private key or callback," but callbacks have no derivation mechanism and no validation enforces that addresses were provided.
Suggested fix
// Connect to RPC
client, err := ethclient.Dial(config.RpcURL)
if err != nil {
return nil, fmt.Errorf("failed to connect to RPC: %w", err)
}
@@
chainID, err := client.NetworkID(ctx)
if err != nil {
+ client.Close()
return nil, fmt.Errorf("failed to get network ID: %w", err)
}
@@
if config.ChainID != 0 && chainID.Int64() != config.ChainID {
+ client.Close()
return nil, fmt.Errorf("chain ID mismatch: expected %d, got %d", config.ChainID, chainID.Int64())
}
@@
if config.Signer != nil {
signer.signer = config.Signer
+ if len(signer.addresses) == 0 {
+ client.Close()
+ return nil, fmt.Errorf("addresses must be provided when using Signer callback")
+ }
} else if config.PrivateKey != "" {
privateKey, err := crypto.HexToECDSA(strings.TrimPrefix(config.PrivateKey, "0x"))
if err != nil {
+ client.Close()
return nil, fmt.Errorf("failed to parse private key: %w", err)
}
@@
} else {
+ client.Close()
return nil, fmt.Errorf("either PrivateKey or SignerCallback must be provided")
}🤖 Prompt for AI Agents
In `@facilitator/evm/signer/evm_signer.go` around lines 59 - 118, NewEVMSigner
leaks the RPC client on several early-return error paths and allows a callback
signer with no addresses; ensure the ethclient.Client is closed on every error
after successful ethclient.Dial by calling client.Close() before each fmt.Errorf
return (references: NewEVMSigner, client), and enforce that when config.Signer
!= nil the config.Addresses (or signer.addresses) is non-empty (return an error
and close client if empty) since callbacks cannot derive addresses; keep the
existing private-key flow (createPrivateKeySigner, privateKey ->
signer.addresses) unchanged but add client.Close() on its failure paths as well.
| // VerifyTypedData verifies an EIP-712 typed data signature. | ||
| // Parameters: | ||
| // - ctx: Context for the verification | ||
| // - address: Expected signer address | ||
| // - domain: EIP-712 domain separator | ||
| // - types: Type definitions for the typed data | ||
| // - primaryType: Primary type name | ||
| // - message: The message data as a map | ||
| // - signature: The signature to verify | ||
| func (s *EVMSigner) VerifyTypedData(ctx context.Context, address string, domain types.TypedDataDomain, types map[string][]types.TypedDataField, primaryType string, message map[string]interface{}, signature []byte) (bool, error) { | ||
| // Convert SDK types to go-ethereum types | ||
| typedData := TypedData{ | ||
| Types: Types{ | ||
| "EIP712Domain": []Type{ | ||
| {Name: "name", Type: "string"}, | ||
| {Name: "version", Type: "string"}, | ||
| {Name: "chainId", Type: "uint256"}, | ||
| {Name: "verifyingContract", Type: "address"}, | ||
| }, | ||
| }, | ||
| PrimaryType: primaryType, | ||
| Domain: TypedDataDomain{ | ||
| Name: domain.Name, | ||
| Version: domain.Version, | ||
| ChainId: domain.ChainID, | ||
| VerifyingContract: domain.VerifyingContract, | ||
| }, | ||
| Message: message, | ||
| } | ||
|
|
||
| // Add custom types | ||
| for typeName, fields := range types { | ||
| if typeName == "EIP712Domain" { | ||
| continue // Skip domain type | ||
| } | ||
| typedData.Types[typeName] = make([]Type, len(fields)) | ||
| for i, field := range fields { | ||
| typedData.Types[typeName][i] = Type{ | ||
| Name: field.Name, | ||
| Type: field.Type, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Hash the typed data | ||
| digest, _, err := HashTypedData(typedData) | ||
| if err != nil { | ||
| return false, fmt.Errorf("failed to hash typed data: %w", err) | ||
| } | ||
|
|
||
| // Recover public key from signature | ||
| sig := make([]byte, len(signature)) | ||
| copy(sig, signature) | ||
|
|
||
| // Adjust V value if needed | ||
| if len(sig) == 65 && sig[64] >= 27 { | ||
| sig[64] -= 27 | ||
| } | ||
|
|
||
| pubKey, err := crypto.SigToPub(digest, sig) | ||
| if err != nil { | ||
| return false, fmt.Errorf("failed to recover public key: %w", err) | ||
| } | ||
|
|
||
| recoveredAddr := crypto.PubkeyToAddress(*pubKey) | ||
| expectedAddr := common.HexToAddress(address) | ||
|
|
||
| return recoveredAddr == expectedAddr, nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "evm_signer.go" -type fRepository: gosuda/x402-facilitator
Length of output: 105
🏁 Script executed:
# Once we find the file, let's examine its content
find . -name "evm_signer.go" -type f -exec wc -l {} \;Repository: gosuda/x402-facilitator
Length of output: 109
🏁 Script executed:
sed -n '267,335p' ./facilitator/evm/signer/evm_signer.goRepository: gosuda/x402-facilitator
Length of output: 2126
🏁 Script executed:
# Check the entire file to understand structure and find HashTypedData
head -100 ./facilitator/evm/signer/evm_signer.goRepository: gosuda/x402-facilitator
Length of output: 3213
🏁 Script executed:
sed -n '485,639p' ./facilitator/evm/signer/evm_signer.goRepository: gosuda/x402-facilitator
Length of output: 4546
🏁 Script executed:
# Also search for where HashTypedData is defined
grep -n "func.*HashTypedData" ./facilitator/evm/signer/evm_signer.goRepository: gosuda/x402-facilitator
Length of output: 136
🏁 Script executed:
# Check if there are any tests for VerifyTypedData to understand expected behavior
find . -name "*test*" -type f -path "*signer*" | head -20Repository: gosuda/x402-facilitator
Length of output: 110
🏁 Script executed:
# Search for imports and usage of apitypes in the codebase
rg "apitypes" ./facilitator/Repository: gosuda/x402-facilitator
Length of output: 49
🏁 Script executed:
# Examine the test file
cat ./facilitator/evm/signer/evm_signer_test.goRepository: gosuda/x402-facilitator
Length of output: 12010
🏁 Script executed:
# Search for any other implementations or references to typed data in the codebase
rg "VerifyTypedData|EIP.712|EIP-712" ./Repository: gosuda/x402-facilitator
Length of output: 1817
🏁 Script executed:
# Check go-ethereum apitypes package information
web_search
go-ethereum apitypes EIP-712 implementationRepository: gosuda/x402-facilitator
Length of output: 167
🌐 Web query:
go-ethereum apitypes EIP-712 typed data implementation
💡 Result:
go-ethereum’s EIP-712 implementation lives in github.com/ethereum/go-ethereum/signer/core/apitypes (package apitypes) and follows EIP-712’s digest rule:
keccak256("\x19\x01" || domainSeparator || hashStruct(message)) [1].
What go-ethereum provides (apitypes)
Key types/functions in apitypes include TypedData, TypedDataDomain, and helpers to compute EIP-712 hashes: EncodeType, TypeHash, EncodeData, HashStruct, and TypedDataAndHash [2]. TypedDataAndHash is the “one-shot” helper to compute the signable hash for EIP-712 typed data [2].
Minimal hashing/signing flow (using go-ethereum primitives)
This is the same flow used by go-ethereum’s signer logic (build domain separator, build message hash, apply \x19\x01 prefix, keccak) [3]:
import (
"fmt"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/signer/core/apitypes"
)
func EIP712Digest(td apitypes.TypedData) ([]byte, error) {
domainSep, err := td.HashStruct("EIP712Domain", td.Domain.Map())
if err != nil { return nil, err }
msgHash, err := td.HashStruct(td.PrimaryType, td.Message)
if err != nil { return nil, err }
raw := []byte(fmt.Sprintf("\x19\x01%s%s", string(domainSep), string(msgHash)))
return crypto.Keccak256(raw), nil
}(Then sign digest with crypto.Sign.)
If you’re using Clef
Clef exposes account_signTypedData for signing “structured data conformant to EIP-712” [4].
Sources: EIP-712 spec [1], go-ethereum apitypes docs/API surface [2], go-ethereum signer hashing pattern showing "\x19\x01" + domainSeparator + typedDataHash [3], Clef API docs [4].
Fix incomplete EIP-712 hashing and missing address padding.
The encoder has two critical issues:
-
Address padding bug (line 519):
verifyingContractis appended as raw 20 bytes instead of 32-byte padded. EIP-712 requires all values to be padded to 32 bytes. This causes the domain hash to mismatch the spec, breaking signature verification even for valid signatures. -
Incomplete type support: The encoder hardcodes domain fields and lacks support for arrays,
bytesN, and dependent/nested types. Many valid EIP-712 payloads will fail.
Switch to github.com/ethereum/go-ethereum/signer/core/apitypes for full compliance, or at minimum fix the padding bug and extend the encoder to handle all Solidity types generically.
Fix address padding
case TypedDataDomain:
// Encode domain fields
encoded = append(encoded, crypto.Keccak256([]byte(v.Name))...)
encoded = append(encoded, crypto.Keccak256([]byte(v.Version))...)
encoded = append(encoded, common.LeftPadBytes(v.ChainId.Bytes(), 32)...)
- encoded = append(encoded, common.HexToAddress(v.VerifyingContract).Bytes()...)
+ encoded = append(encoded, common.LeftPadBytes(common.HexToAddress(v.VerifyingContract).Bytes(), 32)...)🤖 Prompt for AI Agents
In `@facilitator/evm/signer/evm_signer.go` around lines 267 - 335, The
VerifyTypedData flow currently builds TypedData/TypedDataDomain and calls
HashTypedData, but the encoder (HashTypedData and the underlying domain/type
encoder) mis-implements EIP-712 by appending verifyingContract as raw 20 bytes
and not handling arrays, bytesN, or nested types; to fix, replace the custom
encoder with go-ethereum's canonical implementation
(github.com/ethereum/go-ethereum/signer/core/apitypes) when computing the domain
and message hashes inside HashTypedData (or fully refactor HashTypedData to
follow apitypes’ EncodeType/HashStruct logic), and if you must keep the custom
encoder, ensure verifyingContract is encoded as a 32-byte left-padded value and
extend type serialization to generically support arrays, fixed-size bytes
(bytesN), and nested/dependent types per EIP-712 spec so VerifyTypedData
recovers the correct digest and address.
| hash := common.HexToHash(txHash) | ||
|
|
||
| receipt, err := bind.WaitMined(ctx, s.client, ðTypes.Transaction{}) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to wait for transaction: %w", err) | ||
| } | ||
|
|
||
| // Get the actual receipt | ||
| receipt, err = s.client.TransactionReceipt(ctx, hash) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get transaction receipt: %w", err) | ||
| } | ||
|
|
||
| return &types.TransactionReceipt{ | ||
| Status: receipt.Status, | ||
| BlockNumber: receipt.BlockNumber.Uint64(), | ||
| TxHash: receipt.TxHash.Hex(), | ||
| }, nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n facilitator/evm/signer/evm_signer.go | sed -n '400,430p'Repository: gosuda/x402-facilitator
Length of output: 1459
🏁 Script executed:
rg "bind\.WaitMined" --context 5Repository: gosuda/x402-facilitator
Length of output: 892
🏁 Script executed:
rg "WaitForTransactionReceipt" facilitator/evm/signer/evm_signer.go -A 25 -B 5Repository: gosuda/x402-facilitator
Length of output: 1259
🏁 Script executed:
rg "TransactionByHash" facilitator/evm/signer/evm_signer.go -B 2 -A 5Repository: gosuda/x402-facilitator
Length of output: 49
🏁 Script executed:
find_external_knowledge
bind.WaitMined go-ethereum behavior empty transactionRepository: gosuda/x402-facilitator
Length of output: 183
🌐 Web query:
go-ethereum bind.WaitMined signature transaction parameter
💡 Result:
bind.WaitMined takes a signed (or at least hash-identifiable) transaction object:
func WaitMined(ctx context.Context, b bind.DeployBackend, tx *types.Transaction) (*types.Receipt, error)- The
txparameter is*types.Transaction(typically the signedTx you just sent viaSendTransaction).WaitMineduses the transaction’s hash to pollb.TransactionReceipt(...)until it’s mined (orctxis canceled). [1][2] - If you only have the hash, use:
func WaitMinedHash(ctx context.Context, b bind.DeployBackend, hash common.Hash) (*types.Receipt, error)[1]
Sources: [1] (pkg.go.dev bind docs), [2] (DeployBackend interface / what WaitMined relies on).
🏁 Script executed:
rg "WaitMinedHash" facilitator/Repository: gosuda/x402-facilitator
Length of output: 49
🏁 Script executed:
rg "import.*bind" facilitator/evm/signer/evm_signer.go -A 10 -B 10Repository: gosuda/x402-facilitator
Length of output: 49
🏁 Script executed:
head -50 facilitator/evm/signer/evm_signer.goRepository: gosuda/x402-facilitator
Length of output: 1690
🏁 Script executed:
rg "import" facilitator/evm/signer/evm_signer.goRepository: gosuda/x402-facilitator
Length of output: 75
🏁 Script executed:
rg "WaitMinedHash\|TransactionByHash" facilitator/evm/signer/ -iRepository: gosuda/x402-facilitator
Length of output: 49
🏁 Script executed:
python3 << 'EOF'
# Check what happens with an empty transaction hash
import hashlib
# An empty ethTypes.Transaction{} would have these fields as zero/nil
# Let's verify the issue conceptually
empty_tx_fields = {
"Nonce": 0,
"GasPrice": 0,
"Gas": 0,
"To": None,
"Value": 0,
"Data": b'',
"V": 0,
"R": 0,
"S": 0,
}
# An empty transaction would have a deterministic zero hash
print("Empty transaction would have a zero/invalid hash")
print("WaitMined polling for this hash would never succeed")
print("\nCorrect hash from txHash parameter is created but never used with WaitMined")
print("Current flow:")
print("1. hash := common.HexToHash(txHash) - creates correct hash")
print("2. bind.WaitMined(..., ðTypes.Transaction{}) - polls with WRONG (empty) hash")
print("3. s.client.TransactionReceipt(ctx, hash) - finally uses correct hash")
print("\nResult: WaitMined never actually waits for the target transaction")
EOFRepository: gosuda/x402-facilitator
Length of output: 534
🏁 Script executed:
rg "go.mod|go.sum" facilitator/ -lRepository: gosuda/x402-facilitator
Length of output: 49
🏁 Script executed:
cat facilitator/go.mod | grep "ethereum"Repository: gosuda/x402-facilitator
Length of output: 117
🏁 Script executed:
find . -name "go.mod" -type fRepository: gosuda/x402-facilitator
Length of output: 75
🏁 Script executed:
cat go.mod | grep ethereumRepository: gosuda/x402-facilitator
Length of output: 211
🏁 Script executed:
python3 << 'EOF'
# Verify the issue exists and the severity
import subprocess
import json
# The issue is confirmed:
# 1. Empty transaction has no valid hash
# 2. WaitMined waits for wrong hash
# 3. Code will either timeout or skip waiting
# 4. Then fetches receipt anyway (line 413)
print("ISSUE VERIFICATION SUMMARY")
print("=" * 60)
print("✗ bind.WaitMined(ðTypes.Transaction{}) is passed EMPTY tx")
print("✗ Empty tx has zero/invalid hash, not the target txHash")
print("✗ WaitMined will poll for wrong hash - never finds the target")
print("✗ Code then fetches receipt anyway (line 413), bypassing WaitMined")
print("\nIMPACT:")
print("- Unnecessary timeout/delay (waits for non-existent tx)")
print("- Defeats purpose of WaitMined (wait for specific tx)")
print("- TransactionReceipt call is the real work (line 413)")
print("\nSUGGESTED FIX OPTIONS:")
print("1. Fetch tx by hash, pass to WaitMined (current suggestion)")
print("2. Use WaitMinedHash(ctx, client, hash) if available")
EOFRepository: gosuda/x402-facilitator
Length of output: 710
🏁 Script executed:
web_search
go-ethereum v1.16.8 WaitMinedHash availableRepository: gosuda/x402-facilitator
Length of output: 167
WaitMined won't wait for the target transaction—it's getting an empty transaction.
Passing ðTypes.Transaction{} to bind.WaitMined means it has no hash to wait for. The correct hash (created from txHash on line 405) is only used later in the TransactionReceipt call, making the WaitMined call pointless.
Fetch the transaction by hash first, then pass it to WaitMined:
Suggested fix
hash := common.HexToHash(txHash)
- receipt, err := bind.WaitMined(ctx, s.client, ðTypes.Transaction{})
+ tx, _, err := s.client.TransactionByHash(ctx, hash)
+ if err != nil {
+ return nil, fmt.Errorf("failed to get transaction by hash: %w", err)
+ }
+
+ receipt, err := bind.WaitMined(ctx, s.client, tx)
if err != nil {
return nil, fmt.Errorf("failed to wait for transaction: %w", err)
}
-
- // Get the actual receipt
- receipt, err = s.client.TransactionReceipt(ctx, hash)
- if err != nil {
- return nil, fmt.Errorf("failed to get transaction receipt: %w", err)
- }
return &types.TransactionReceipt{
Status: receipt.Status,🤖 Prompt for AI Agents
In `@facilitator/evm/signer/evm_signer.go` around lines 405 - 423, The call to
bind.WaitMined is using an empty ethTypes.Transaction so it never waits for the
real tx; instead, first fetch the transaction by hash (use the existing hash
variable created from txHash and s.client.TransactionByHash or
s.client.TransactionReceipt lookup) to obtain the *ethTypes.Transaction, then
pass that transaction to bind.WaitMined(ctx, s.client, tx) before calling
s.client.TransactionReceipt(ctx, hash); update the flow in evm_signer.go around
the hash variable, bind.WaitMined, and TransactionReceipt calls so WaitMined
receives the actual transaction object.
| func NewFacilitator(network string, url string, privateKeyHex string) (*Facilitator, error) { | ||
| client := client.NewClient(url) |
There was a problem hiding this comment.
Variable shadowing: client shadows the imported package.
Line 22 declares client := client.NewClient(url) which shadows the imported client package. Rename the variable to avoid confusion.
-func NewFacilitator(network string, url string, privateKeyHex string) (*Facilitator, error) {
- client := client.NewClient(url)
+func NewFacilitator(network string, url string, privateKeyHex string) (*Facilitator, error) {
+ solClient := client.NewClient(url)Then update line 36:
- client: client,
+ client: solClient,🤖 Prompt for AI Agents
In `@facilitator/solana/facilitator.go` around lines 21 - 22, The local variable
in NewFacilitator shadows the imported client package; rename the local variable
(e.g., from client to solClient or remoteClient) when assigning
client.NewClient(url) and update all subsequent references inside NewFacilitator
(including the later use at the line currently flagged) to that new name so the
imported package remains accessible.
| func NewFacilitator(network string, url string, privateKeyHex string) (*Facilitator, error) { | ||
| client := client.NewClient(url) | ||
|
|
||
| privKey, err := hex.DecodeString(privateKeyHex) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("invalid hex private key: %w", err) | ||
| } | ||
|
|
||
| feePayer, err := solTypes.AccountFromBytes(privKey) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("invalid private key format: %w", err) | ||
| } | ||
|
|
||
| return &Facilitator{ | ||
| scheme: types.Solana, | ||
| client: client, | ||
| feePayer: feePayer, | ||
| }, nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the Solana facilitator to see if network is used anywhere
cd facilitator/solana || exit 1
rg "network" --max-count 20 facilitator.goRepository: gosuda/x402-facilitator
Length of output: 391
🏁 Script executed:
#!/bin/bash
# Look for other facilitator implementations to understand the pattern
fd "facilitator.go" facilitator/ -type fRepository: gosuda/x402-facilitator
Length of output: 237
🏁 Script executed:
#!/bin/bash
# Check if there are other NewFacilitator functions in the codebase
ast-grep --pattern $'func NewFacilitator($$$) ($_, $_) {
$$$
}'Repository: gosuda/x402-facilitator
Length of output: 5086
🏁 Script executed:
#!/bin/bash
# Check the Facilitator struct definition in solana package
rg "type Facilitator struct" facilitator/solana/ -A 10Repository: gosuda/x402-facilitator
Length of output: 738
Remove or use the network parameter consistently.
The network parameter is accepted but never used in the constructor or stored in the struct. Unlike the EVM facilitator, which validates and stores the network parameter, the Solana implementation ignores it entirely. Either remove the parameter or adopt the EVM pattern (validate and store it) to make it available to GetExtra/GetSigners methods.
🤖 Prompt for AI Agents
In `@facilitator/solana/facilitator.go` around lines 21 - 39, NewFacilitator
currently accepts a network string but doesn't use or store it; update the
Facilitator implementation to follow the EVM pattern by validating the network
value (e.g., allowed Solana networks like "mainnet", "testnet", "devnet" or your
project's canonical set) and add a network field to the Facilitator struct, then
assign the validated value in NewFacilitator; ensure downstream methods that
reference network (GetExtra, GetSigners) read the stored Facilitator.network.
Alternatively, if you prefer to drop the parameter, remove the network argument
from NewFacilitator and all callers, but do not leave an unused parameter;
update NewFacilitator, the Facilitator struct, and any callers accordingly.
| type Facilitator struct { | ||
| } | ||
|
|
||
| func NewFacilitator(network string, url string, privateKeyHex string) (*Facilitator, error) { | ||
| return &Facilitator{}, nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Constructor discards all parameters; struct has no fields.
The Facilitator struct is empty, and NewFacilitator ignores network, url, and privateKeyHex entirely. Compare with facilitator/solana/facilitator.go (lines 14-18, 20-38) which stores scheme, client, and feePayer, and validates the private key. Similarly, the EVM facilitator stores scheme and network and validates all inputs.
Even for a stub, you should:
- Store the parameters for when implementation is added
- Validate inputs to fail fast on misconfiguration
Proposed structure to align with other facilitators
type Facilitator struct {
+ scheme types.Scheme
+ network string
+ rpcURL string
+ privateKey string // or parsed key type when implementing
}
func NewFacilitator(network string, url string, privateKeyHex string) (*Facilitator, error) {
- return &Facilitator{}, nil
+ if network == "" {
+ return nil, fmt.Errorf("network is required")
+ }
+ if url == "" {
+ return nil, fmt.Errorf("rpc URL is required")
+ }
+ if privateKeyHex == "" {
+ return nil, fmt.Errorf("private key is required")
+ }
+ return &Facilitator{
+ scheme: types.Sui,
+ network: network,
+ rpcURL: url,
+ privateKey: privateKeyHex,
+ }, nil
}🤖 Prompt for AI Agents
In `@facilitator/sui/facilitator.go` around lines 12 - 17, The Facilitator
constructor currently drops all inputs and the struct is empty; update the
Facilitator type to include fields for network, url, and privateKeyHex (or
parsed/validated equivalents like scheme/client/privateKey) and modify
NewFacilitator to validate parameters (non-empty network, valid URL format, and
non-empty/hex-valid privateKeyHex) before returning the instance; ensure
NewFacilitator stores the values into the Facilitator fields (and if applicable,
parse/validate the private key similarly to the Solana/EVM facilitators and
return an error on invalid input) so callers fail fast and the struct retains
the provided configuration.
| func NewFacilitator(network string, url string, privateKeyHex string) (*Facilitator, error) { | ||
| return &Facilitator{}, nil | ||
| } | ||
|
|
||
| func (t *Facilitator) Verify(ctx context.Context, payload x402types.PaymentPayload, req x402types.PaymentRequirements) (*types.VerifyResponse, error) { | ||
| return nil, fmt.Errorf("tron verify not implemented") | ||
| } | ||
|
|
||
| func (t *Facilitator) Settle(ctx context.Context, payload x402types.PaymentPayload, req x402types.PaymentRequirements) (*types.SettleResponse, error) { | ||
| return nil, fmt.Errorf("tron settle not implemented") | ||
| } |
There was a problem hiding this comment.
Fail fast until Tron implementation exists.
NewFacilitator returns a usable instance while Verify/Settle always fail. That allows the server to boot and only fail at runtime. Prefer returning an error up front (or fully implement and store config).
✅ Suggested fix
func NewFacilitator(network string, url string, privateKeyHex string) (*Facilitator, error) {
- return &Facilitator{}, nil
+ return nil, fmt.Errorf("tron facilitator not implemented")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func NewFacilitator(network string, url string, privateKeyHex string) (*Facilitator, error) { | |
| return &Facilitator{}, nil | |
| } | |
| func (t *Facilitator) Verify(ctx context.Context, payload x402types.PaymentPayload, req x402types.PaymentRequirements) (*types.VerifyResponse, error) { | |
| return nil, fmt.Errorf("tron verify not implemented") | |
| } | |
| func (t *Facilitator) Settle(ctx context.Context, payload x402types.PaymentPayload, req x402types.PaymentRequirements) (*types.SettleResponse, error) { | |
| return nil, fmt.Errorf("tron settle not implemented") | |
| } | |
| func NewFacilitator(network string, url string, privateKeyHex string) (*Facilitator, error) { | |
| return nil, fmt.Errorf("tron facilitator not implemented") | |
| } | |
| func (t *Facilitator) Verify(ctx context.Context, payload x402types.PaymentPayload, req x402types.PaymentRequirements) (*types.VerifyResponse, error) { | |
| return nil, fmt.Errorf("tron verify not implemented") | |
| } | |
| func (t *Facilitator) Settle(ctx context.Context, payload x402types.PaymentPayload, req x402types.PaymentRequirements) (*types.SettleResponse, error) { | |
| return nil, fmt.Errorf("tron settle not implemented") | |
| } |
🤖 Prompt for AI Agents
In `@facilitator/tron/facilitator.go` around lines 15 - 25, NewFacilitator
currently returns a usable Facilitator while Verify and Settle always error,
allowing the server to start but fail at runtime; change NewFacilitator to
fail-fast by returning an error until the Tron implementation is complete (or
alternatively fully construct and store required config on the Facilitator).
Specifically, update NewFacilitator to validate inputs (network, url,
privateKeyHex) and return a descriptive error (e.g., "tron facilitator not
implemented" or input validation errors) instead of &Facilitator{}, so callers
cannot obtain a non-functional Facilitator instance; keep Verify and Settle
unchanged until implemented or implement them together with proper
initialization.
1 participant
