feat: switch gasfree API to BankOfAI proxy#61
Conversation
Replace direct gasfree.io API endpoints with BankOfAI official proxy at facilitator.bankofai.io which handles authentication transparently, removing the need for clients to manage API keys/secrets and HMAC signatures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code Review ReportProject: x402 (BankOfAI SDK) PR OverviewBranch Information
Commit History
Review SummaryVerdict
Findings at a Glance
SummaryThis PR re-routes all GasFree API traffic from the upstream However, the PR carries a critical architectural trust concern: user-signed financial transactions — TIP-712 typed signatures authorising GasFree token transfers — are now submitted through a BankOfAI-controlled proxy with no authentication, integrity tokens, or fallback mechanism. Any outage of Change Summary1 — GasFree Proxy URL Switch (Python & TypeScript config)
Purpose: Route GasFree API calls through the BankOfAI-operated proxy instead of the upstream 2 — Authentication Removal (Python & TypeScript gasfree client)
Purpose: Since the BankOfAI proxy does not require signed API credentials, all HMAC generation and 3 — Config Helper Changes (Python & TypeScript config)
Purpose: Harden the configuration by failing fast on unsupported networks and removing now-unnecessary credential helpers. 4 — Test Updates
Purpose: Align tests with the simplified constructor and header logic. Detailed FindingsCritical[C-01] Signed User Transactions Routed Through Unauthenticated 3rd-Party Proxy
Description All GasFree API calls — including the
Code # python/x402/src/bankofai/x402/config.py
GASFREE_API_BASE_URLS: Dict[str, str] = {
"tron:mainnet": "https://facilitator.bankofai.io/mainnet",
"tron:shasta": "https://facilitator.bankofai.io/shasta",
"tron:nile": "https://facilitator.bankofai.io/nile",
}// typescript/packages/x402/src/utils/gasfree.ts
private getHeaders(): Record<string, string> {
return {
'Content-Type': 'application/json',
// No Authorization, no Timestamp
};
}Recommendation Before merging, document and address the following:
Major[MJ-01] Breaking Change:
|
| Property | Value |
|---|---|
| Severity | Major |
| Category | Compatibility |
| File | python/x402/src/bankofai/x402/utils/gasfree.py : Lines 38–39; typescript/packages/x402/src/utils/gasfree.ts : Lines 83–85 |
Description
The GasFreeAPIClient constructor previously accepted optional api_key/api_secret (Python) and apiKey/apiSecret (TypeScript) parameters. Any SDK consumer that constructed the client with their own credentials — a supported and documented pattern — will now receive a TypeError (Python: unexpected keyword argument; TypeScript: compile-time error if typed, silent extra arg if not).
Code
# Before (main branch)
def __init__(self, base_url: str, api_key: str | None = None, api_secret: str | None = None):
# After (feature branch)
def __init__(self, base_url: str):// Before
constructor(baseUrl: string, apiKey?: string, apiSecret?: string)
// After
constructor(baseUrl: string)Recommendation
# Option A: Retain params as no-ops with a deprecation warning
def __init__(self, base_url: str, api_key: str | None = None, api_secret: str | None = None):
if api_key or api_secret:
import warnings
warnings.warn(
"api_key and api_secret are deprecated and ignored by the BankOfAI proxy client.",
DeprecationWarning, stacklevel=2
)
self.base_url = base_url.rstrip("/")// Option A: TypeScript deprecation
constructor(baseUrl: string, /** @deprecated */ apiKey?: string, /** @deprecated */ apiSecret?: string) {
if (apiKey || apiSecret) {
console.warn('[x402] GasFreeAPIClient: apiKey/apiSecret are deprecated and ignored.');
}
this.baseUrl = baseUrl.replace(/\/$/, '');
}[MJ-02] Breaking Change: getGasFreeApiKey() and getGasFreeApiSecret() Removed From Public API
| Property | Value |
|---|---|
| Severity | Major |
| Category | Compatibility |
| File | python/x402/src/bankofai/x402/config.py (removed Lines 136–156); typescript/packages/x402/src/config.ts (removed Lines ~183–209) |
Description
getGasFreeApiKey() / get_gasfree_api_key() and getGasFreeApiSecret() / get_gasfree_api_secret() were exported as part of the public module surface. The TypeScript SDK re-exports everything from config.ts via index.ts → export * from './config.js', so these functions were consumable by SDK users. Removing them without a deprecation cycle can silently break downstream integrations that reference them at runtime.
Code
// Removed entirely from config.ts (was exported via index.ts)
export function getGasFreeApiKey(network?: string): string | undefined { ... }
export function getGasFreeApiSecret(network?: string): string | undefined { ... }Recommendation
Keep the function stubs for one minor version cycle, decorated with @deprecated / console.warn, and document the removal in the CHANGELOG under a ### Removed heading. If the intent is an immediate major-version bump, mark it clearly in the PR and version files.
[MJ-03] Breaking Behavior Change: getGasFreeApiBaseUrl() Now Throws on Unknown Network
| Property | Value |
|---|---|
| Severity | Major |
| Category | Correctness / Compatibility |
| File | python/x402/src/bankofai/x402/config.py : Lines 127–132; typescript/packages/x402/src/config.ts : Lines 173–179 |
Description
Previously the function returned a default fallback URL ("https://api.gasfree.io/v1" in TypeScript; similar in Python) for any network not found in the map. After this PR it raises UnsupportedNetworkError. Callers that passed arbitrary network strings and handled the returned URL defensively will now encounter an uncaught exception. While the new fail-fast behaviour is arguably better design, it is a silent breaking change.
Code
// Before
export function getGasFreeApiBaseUrl(network: string): string {
return GASFREE_API_BASE_URLS[network] ?? 'https://api.gasfree.io/v1';
}
// After
export function getGasFreeApiBaseUrl(network: string): string {
const url = GASFREE_API_BASE_URLS[network];
if (!url) {
throw new UnsupportedNetworkError(`No GasFree API URL configured for network: ${network}`);
}
return url;
}Recommendation
Document the behavioral change in the CHANGELOG. Update the function's JSDoc/docstring to reflect that it now throws. Ensure all internal call-sites within the SDK handle UnsupportedNetworkError appropriately (a quick audit shows they already do via the ExactGasFreeClientMechanism, which throws its own error if no client is registered for the network).
[MJ-04] No CHANGELOG Entry for an Infrastructure-Level Change
| Property | Value |
|---|---|
| Severity | Major |
| Category | Documentation |
| File | CHANGELOG.md |
Description
The CHANGELOG's most recent entry ([0.5.7]) documents the TronGrid fallback RPC feature. This PR introduces a far more significant change — switching every GasFree API endpoint from the upstream gasfree.io provider to the BankOfAI proxy — but adds no CHANGELOG entry and does not bump the version. Operators who rely on environment-based API key configuration (GASFREE_API_KEY, GASFREE_API_SECRET) will have their credentials silently ignored after upgrading, with no upgrade notice.
Code
# CHANGELOG.md — no entry for the gasfree proxy switch
## [0.5.7] - 2026-03-30
### Added
- Fallback RPC endpoint ... (unrelated to this PR)Recommendation
Add a ## [0.5.8] (or appropriate next version) section with, at minimum:
### Changed
- GasFree API endpoints now route through the BankOfAI proxy (`https://facilitator.bankofai.io/{mainnet,shasta,nile}`)
instead of the upstream `open.gasfree.io` / `open-test.gasfree.io` endpoints.
- `getGasFreeApiBaseUrl()` / `get_gasfree_api_base_url()` now throws `UnsupportedNetworkError`
for unrecognised networks instead of returning a default fallback URL.
### Removed
- `getGasFreeApiKey()` / `get_gasfree_api_key()` — credentials are no longer required by the proxy.
- `getGasFreeApiSecret()` / `get_gasfree_api_secret()` — same.
- `GasFreeAPIClient` `apiKey`/`apiSecret` constructor parameters (Python: `api_key`/`api_secret`).Minor
[MN-01] Test Directly Calls Private Method _get_headers()
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Testing |
| File | python/x402/tests/utils/test_gasfree_utils.py : Lines 13–18 |
Description
The test_get_headers test accesses client._get_headers(), which is a private method by Python convention (single-underscore prefix). Directly testing private implementation details couples the test to the implementation, making refactors unnecessarily risky. The same assertion could be validated indirectly through test_get_address_info by inspecting the headers passed to the HTTP mock.
Recommendation
Either remove the test (the headers are implicitly verified by the other HTTP call tests) or verify via the mock:
# Verify headers indirectly in test_get_address_info
with patch("httpx.AsyncClient.get") as mock_get:
...
call_kwargs = mock_get.call_args.kwargs
assert call_kwargs["headers"]["Content-Type"] == "application/json"
assert "Authorization" not in call_kwargs["headers"][MN-02] Residual Dead Method: _get_headers() / getHeaders() is a One-Liner Wrapper
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Code Quality |
| File | python/x402/src/bankofai/x402/utils/gasfree.py : Lines 41–43; typescript/packages/x402/src/utils/gasfree.ts : Lines 90–93 |
Description
After the simplification, _get_headers() / getHeaders() returns only {"Content-Type": "application/json"}. The method now exists purely as a thin wrapper with no logic, called in four places (Python) and four places (TypeScript). This adds a layer of indirection with no benefit. The only upside is future-proofing (e.g., adding bearer tokens later), but without a clear near-term plan this is dead abstraction.
Recommendation
If the proxy is expected to require authentication headers in a future iteration, retain the method and add a comment explaining the intent. Otherwise inline the dict at each call site for clarity. Either choice is acceptable; the inconsistency between the method's name implying complexity and its trivial body is the issue worth noting.
Suggestions
[S-01] No Health-Check or Connectivity Test for New Proxy Endpoint
File: python/x402/tests/, typescript/packages/x402/src/utils/gasfree.test.ts
Description: The previous endpoints (open.gasfree.io) were well-known upstream services. The new facilitator.bankofai.io endpoint is self-operated. There are no tests — even a simple integration/smoke test guarded by an environment flag — that verify the proxy is reachable and returns expected response shapes.
Suggestion: Add a conditional integration test (run only when RUN_INTEGRATION_TESTS=1 is set) that makes a live call to GET /api/v1/config/provider/all against facilitator.bankofai.io/mainnet and asserts a 200 response with a non-empty providers array. This would catch proxy misconfiguration before a release.
[S-02] Stale URL Detection Logic in Old __init__ Code Would Have Failed for New URLs
File: typescript/packages/x402/src/utils/gasfree.ts (removed code); python/x402/src/bankofai/x402/utils/gasfree.py (removed code)
Description: The removed constructor logic inferred the network from the base URL by checking for "open-test" in the URL string. This heuristic would have incorrectly mapped facilitator.bankofai.io/shasta to tron:mainnet (no "open-test" substring), confirming the old detection code was already fragile. Its removal is the correct action.
Suggestion: Document in a code comment why the network-from-URL inference approach was abandoned, as a record for future contributors who might be tempted to re-introduce it.
Positive Observations
| Area | Observation |
|---|---|
| Code simplification | The PR achieves a net -196 lines of logic across two languages by removing HMAC scaffolding that is no longer needed. The resulting client classes are significantly easier to read and test. |
| Consistent Python/TypeScript parity | Both SDK implementations make identical changes in lockstep, maintaining the project's cross-language consistency. |
| Fail-fast on unsupported networks | Changing getGasFreeApiBaseUrl() to throw instead of silently returning a wrong default URL is the correct design direction; errors at configuration time are far better than mysterious runtime failures. |
| Removal of debug log leaking auth material | The old TypeScript generateSignature() contained console.debug(GasFree HMAC base string: ${message}), which could leak auth material to logs. Its removal is a positive security improvement. |
| Test assertions updated correctly | The Python and TypeScript tests correctly assert the absence of Authorization and Timestamp headers, not just the presence of Content-Type. |
Checklist Results
| Category | Items Checked | Pass | Fail | N/A | Notes |
|---|---|---|---|---|---|
| Correctness | 8 | 7 | 1 | 0 | getGasFreeApiBaseUrl behavior change is correct per new design but breaks existing callers |
| Security | 10 | 6 | 2 | 2 | Unauthenticated proxy for financial transactions; no fallback if proxy is unavailable |
| Performance | 7 | 7 | 0 | 0 | No regressions introduced |
| Code Quality | 10 | 8 | 2 | 0 | Dead wrapper method; test accesses private method |
| Testing | 7 | 5 | 2 | 0 | Missing integration test for new endpoint; private-method test smell |
| Documentation | 6 | 2 | 4 | 0 | CHANGELOG not updated; breaking changes undocumented; no migration guide |
| Compatibility | 5 | 1 | 4 | 0 | Three breaking public-API changes + one behavioral change without deprecation |
| Observability | 4 | 4 | 0 | 0 | Existing error logging retained; accidental debug log removed |
Disclaimer
This is an automated code review. It supplements but does not replace human review. The reviewer analysed only the diff between main and feature/bankofai-gasfree-proxy. Runtime behaviour of facilitator.bankofai.io, proxy-level authentication mechanisms, and deployment infrastructure are not verifiable from the source code alone.
Report generated by Code Review Skill v1.0.0
Date: 2026-03-30
- Add deprecation warnings for GasFreeAPIClient api_key/api_secret params - Add CHANGELOG entry for gasfree proxy migration - Update RELEASE_NOTES.md for v0.5.8 - Bump version to 0.5.8 in all package files - Fix ruff formatting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code Review Audit ReportRepository: BofAI/x402 PR Overview
Commit History
Review SummaryVerdict: Request Changes The core change — routing GasFree API calls through the BankOfAI proxy and removing HMAC authentication — is architecturally sound and well-documented. The implementation is clean and the test suites are thorough for the happy-path and polling scenarios. However, there are two breaking API removals without backward-compatible shims ( Findings at a Glance
Change Summary1. GasFree API routing (core feature)
2. Config helpers removed
3. Proxy URL constants
4. Mechanism import cleanup
5. Dependency lock file churn
6. Version bumps and docs
Detailed Findings[C-01] Hard deletion of public API without deprecation shim (Critical)Category: Correctness / Breaking Change
Description: Any downstream code that calls these functions will receive a Code (main branch — now missing): // config.ts (deleted in this PR)
export function getGasFreeApiKey(network?: string): string | undefined { ... }
export function getGasFreeApiSecret(network?: string): string | undefined { ... }Recommendation: /** @deprecated No longer needed — removed in 0.5.8 */
export function getGasFreeApiKey(_network?: string): string | undefined {
console.warn('[x402] getGasFreeApiKey is removed as of 0.5.8 and always returns undefined.');
return undefined;
}Mirror in Python with [MJ-01] Implicit single-proxy trust without documented security model (Major)Category: Security / Architecture
Description:
For a payment SDK, the implicit trust expansion to a single third-party proxy is a non-trivial security surface change. Code: export const GASFREE_API_BASE_URLS: Record<string, string> = {
'tron:mainnet': 'https://facilitator.bankofai.io/mainnet',
'tron:shasta': 'https://facilitator.bankofai.io/shasta',
'tron:nile': 'https://facilitator.bankofai.io/nile',
};Recommendation:
[MJ-02] Python
|
| Check | Status | Notes |
|---|---|---|
| All changed files read | Pass | |
| Breaking changes documented | Partial | CHANGELOG/RELEASE_NOTES document the break; no in-code shim provided |
| Deprecation path for removed APIs | Fail | Functions deleted without stub — see C-01 |
| Unit tests updated | Pass | New tests added for proxy-auth-free paths |
| Error handling complete | Pass | Both SDKs guard null data, HTTP errors, and missing fields |
| No secrets or credentials committed | Pass | |
| Type safety maintained | Partial | domain: any, message: any in TS submit() — see MN-01 |
| Performance implications considered | Partial | No connection pooling in Python — see MJ-02 |
| Security model documented | Fail | Proxy trust model not documented — see MJ-01 |
| Version bumps consistent | Pass | Both SDKs at 0.5.8 |
| Lock files regenerated cleanly | Pass | |
| No debug/test code left in production | Pass |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code Audit Report: PR
|
| Property | Value |
|---|---|
| Base Branch | origin/main |
| Feature Branch | origin/feature/bankofai-gasfree-proxy |
| Version Bump | 0.5.7 → 0.5.8 |
| Release Date | 2026-03-30 |
Commit History
| Hash | Message |
|---|---|
35d5b5d |
refactor: remove deprecation stubs, clean break for gasfree auth removal |
0e10f0e |
chore: bump SDK versions to 0.5.8, add deprecation stubs and changelog |
a9cf5bc |
feat: switch gasfree API to BankOfAI proxy, remove API key/secret auth |
Files Changed (15 files, +1514 / -1707 lines)
| File | Change Type | Category |
|---|---|---|
CHANGELOG.md |
Modified | Documentation |
RELEASE_NOTES.md |
Modified | Documentation |
python/x402/pyproject.toml |
Modified | Config |
python/x402/src/bankofai/x402/__init__.py |
Modified | Config (version bump) |
python/x402/src/bankofai/x402/config.py |
Modified | Refactor |
python/x402/src/bankofai/x402/utils/gasfree.py |
Modified | Feature/Refactor |
python/x402/tests/utils/test_gasfree_utils.py |
Modified | Test |
python/x402/uv.lock |
Modified | Dependency Lock |
typescript/package-lock.json |
Modified | Dependency Lock |
typescript/package.json |
Modified | Config |
typescript/packages/x402/package.json |
Modified | Config |
typescript/packages/x402/src/config.ts |
Modified | Refactor |
typescript/packages/x402/src/mechanisms/exactGasfree.ts |
Modified | Refactor |
typescript/packages/x402/src/utils/gasfree.test.ts |
Modified | Test |
typescript/packages/x402/src/utils/gasfree.ts |
Modified | Feature/Refactor |
2. Change Summary
2.1 GasFree API Endpoint Migration
The core change routes all GasFree API calls through the BankOfAI proxy (https://facilitator.bankofai.io/{mainnet,shasta,nile}) instead of the upstream open.gasfree.io / open-test.gasfree.io endpoints. This eliminates the need for clients to configure GASFREE_API_KEY and GASFREE_API_SECRET environment variables.
2.2 HMAC-SHA256 Authentication Removal
The GasFreeAPIClient in both Python and TypeScript previously generated Authorization: ApiKey <key>:<HMAC-SHA256-sig> and Timestamp headers for every request. These are completely removed. The BankOfAI proxy is expected to handle upstream authentication transparently.
2.3 Fail-Fast on Unsupported Networks
getGasFreeApiBaseUrl() (TypeScript) and get_gasfree_api_base_url() (Python) previously returned a hardcoded fallback URL (https://api.gasfree.io/v1) for unknown networks. Both now throw UnsupportedNetworkError instead, preventing silent misconfiguration.
2.4 Public API Removal (Breaking Change)
The following public functions are removed from both SDKs:
getGasFreeApiKey()/get_gasfree_api_key()getGasFreeApiSecret()/get_gasfree_api_secret()GasFreeAPIClientconstructor parametersapi_key/apiKeyandapi_secret/apiSecret
2.5 Version Bump and Documentation
Both Python (bankofai-x402) and TypeScript (@bankofai/x402) packages are bumped from 0.5.7 to 0.5.8. CHANGELOG and RELEASE_NOTES are updated accordingly.
2.6 Test Cleanup
Tests that exercised HMAC signature generation and header injection are removed/replaced with simpler header-presence assertions. New test cases confirming the absence of Authorization and Timestamp headers are added.
3. Detailed Findings
[MAJOR-01] TypeScript requestId Always Strips Two Characters Regardless of Signature Format
| Property | Value |
|---|---|
| Severity | Major |
| Category | Correctness & Logic |
| File | typescript/packages/x402/src/utils/gasfree.ts : Lines 250-251 |
Description:
In the submit() method, sig is conditionally stripped of its 0x prefix, but requestId unconditionally calls signature.slice(2, 10) — always discarding the first two characters of the original signature regardless of format. If the signature does not start with 0x, the requestId will be derived from different bytes than sig, making the two values inconsistent with each other and potentially causing unexpected request ID collisions or confusing server-side logs.
In Python, the equivalent code correctly slices sig (already stripped of 0x) using sig[:8]. The TypeScript code does not match this behaviour.
Code:
// gasfree.ts line 250-251
sig: signature.startsWith('0x') ? signature.slice(2) : signature,
requestId: `x402-${Date.now()}-${signature.slice(2, 10)}`,
// ^^^^^^^^^^^^^^^^^^^^^^
// BUG: Always slices position 2-10 of the *original* signature,
// not of the already-stripped `sig` valueRecommendation:
Derive requestId from the already-normalised sig value:
const sig = signature.startsWith('0x') ? signature.slice(2) : signature;
const payload = {
...
sig,
requestId: `x402-${Date.now()}-${sig.slice(0, 8)}`,
};[MAJOR-02] No HTTP Request Timeout on Individual API Calls (Python)
| Property | Value |
|---|---|
| Severity | Major |
| Category | Correctness & Logic / Performance |
| File | python/x402/src/bankofai/x402/utils/gasfree.py : Lines 48, 73, 109, 188 |
Description:
Every method in GasFreeAPIClient (get_address_info, get_providers, get_status, submit) creates a new httpx.AsyncClient() with no timeout configured. The default httpx timeout is 5 seconds for connection but no overall timeout for read. If the BankOfAI proxy is slow or unresponsive, these calls can hang indefinitely, blocking the entire payment flow or exhausting the event loop.
The TypeScript client relies on the browser/Node fetch API which also has no default timeout — the same concern applies there, though it is harder to configure uniformly.
Code:
# gasfree.py line 48
async with httpx.AsyncClient() as client: # no timeout= argument
response = await client.get(url, headers=headers)Recommendation:
Configure an explicit timeout on each httpx.AsyncClient:
async with httpx.AsyncClient(timeout=httpx.Timeout(10.0)) as client:For TypeScript, use AbortController with a timeout signal:
const controller = new AbortController();
const id = setTimeout(() => controller.abort(), 10_000);
const response = await fetch(url, { headers, signal: controller.signal });
clearTimeout(id);[MAJOR-03] New Proxy Endpoint Introduces Single Point of Failure with No Fallback
| Property | Value |
|---|---|
| Severity | Major |
| Category | Correctness & Logic / Security |
| File | python/x402/src/bankofai/x402/config.py : Lines 53-57, typescript/packages/x402/src/config.ts : Lines 59-63 |
Description:
All three network environments (mainnet, shasta, nile) now route exclusively through https://facilitator.bankofai.io. The upstream GasFree API (open.gasfree.io) is completely removed as a fallback. If the BankOfAI proxy experiences an outage, all GasFree payment flows across all environments will fail with no recovery path for operators. The previous code at least had a degraded-fallback URL; the current design has no mitigation.
Additionally, the proxy URL is fully hardcoded in the SDK binary — operators cannot override it via configuration or environment variables, making it impossible to point to an alternative relay without rebuilding from source.
Code:
GASFREE_API_BASE_URLS: Dict[str, str] = {
"tron:mainnet": "https://facilitator.bankofai.io/mainnet",
"tron:shasta": "https://facilitator.bankofai.io/shasta",
"tron:nile": "https://facilitator.bankofai.io/nile",
}Recommendation:
- Allow the base URL to be overridden via environment variables (e.g.,
GASFREE_API_BASE_URL_MAINNET,GASFREE_API_BASE_URL). - Document the proxy SLA and outage procedures in the README.
- Consider keeping the upstream URL as a documented fallback that advanced operators can configure.
[MINOR-01] Missing Tests for New getGasFreeApiBaseUrl / get_gasfree_api_base_url Behaviour
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Testing |
| File | python/x402/tests/test_config.py, typescript/packages/x402/src/config.test.ts |
Description:
Both Python and TypeScript introduce a breaking behaviour change for getGasFreeApiBaseUrl / get_gasfree_api_base_url: unknown networks now throw UnsupportedNetworkError instead of returning a fallback URL. The Python config test (tests/test_config.py) only tests the get_gasfree_controller_address method and does not test the new error-throwing behaviour or verify the new BankOfAI URLs. The TypeScript config.test.ts is entirely focused on RPC URL resolution and does not cover getGasFreeApiBaseUrl at all.
Code:
# tests/test_config.py - entire file content
from bankofai.x402.config import NetworkConfig
def test_gasfree_controller_mainnet_address() -> None:
assert (
NetworkConfig.get_gasfree_controller_address("tron:mainnet")
== "TFFAMQLZybALaLb4uxHA9RBE7pxhUAjF3U"
)
# No test for get_gasfree_api_base_url at allRecommendation:
Add tests for:
- Each known network returns the correct BankOfAI proxy URL.
- An unknown network raises
UnsupportedNetworkError(replaces the old silent fallback).
def test_get_gasfree_api_base_url_mainnet():
url = NetworkConfig.get_gasfree_api_base_url("tron:mainnet")
assert url == "https://facilitator.bankofai.io/mainnet"
def test_get_gasfree_api_base_url_unknown_network_raises():
with pytest.raises(UnsupportedNetworkError):
NetworkConfig.get_gasfree_api_base_url("tron:unknown")[MINOR-02] submit() Method Uses any Type for domain and message Parameters (TypeScript)
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Code Quality |
| File | typescript/packages/x402/src/utils/gasfree.ts : Line 236 |
Description:
The submit() method signature uses any for both domain and message parameters. Since the GasFree message structure is fully defined by GasFreeSubmitResponseData and the existing GASFREE_TYPES constant, a proper typed interface exists but is not used here. This weakens compile-time safety and allows callers to silently pass malformed payloads.
Code:
// gasfree.ts line 236
async submit(domain: any, message: any, signature: string): Promise<string> {Recommendation:
Define and use a typed interface for the message parameter. The domain parameter is not used in the method body at all (it is not included in the API payload), so it can be removed from the signature entirely, or replaced with the correct typed object.
export interface GasFreeSubmitMessage {
token: string;
serviceProvider: string;
user: string;
receiver: string;
value: bigint | string | number;
maxFee: bigint | string | number;
deadline: number | bigint;
version: number;
nonce: number;
}
async submit(message: GasFreeSubmitMessage, signature: string): Promise<string>[MINOR-03] domain Parameter in submit() is Accepted But Never Used (Both Languages)
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Code Quality / Dead Code |
| File | typescript/packages/x402/src/utils/gasfree.ts : Line 236, python/x402/src/bankofai/x402/utils/gasfree.py : Line ~183 |
Description:
The submit() method in both Python and TypeScript accepts a domain parameter (which carries the EIP-712 domain for signing verification), but the parameter is never read or included in the API payload. The proxy determines domain context from the base_url path. Keeping the parameter in the public API is misleading and increases the surface area of the interface unnecessarily.
Code:
// TypeScript - domain is accepted but never referenced in the function body
async submit(domain: any, message: any, signature: string): Promise<string> {
const path = '/api/v1/gasfree/submit';
const payload = {
token: message.token,
// ... domain is not used anywhere
};# Python
async def submit(self, domain: Dict[str, Any], message: Dict[str, Any], signature: str) -> str:
# domain is not read or forwarded in payloadRecommendation:
Remove the domain parameter from both submit() methods, or document it explicitly with a deprecation notice if it is kept for future use. If it is intentionally kept for caller compatibility, add an inline # noqa or // eslint-disable comment with an explanation.
[MINOR-04] Error Message Inconsistency: 'id' field vs id field Between Python and TypeScript
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Code Quality / Documentation |
| File | python/x402/src/bankofai/x402/utils/gasfree.py : Line ~221, typescript/packages/x402/src/utils/gasfree.ts : Line 278 |
Description:
The error message for missing id in the submit response differs slightly between the two implementations:
- Python:
"GasFree submit API returned data without 'id' field" - TypeScript:
"GasFree submit API returned data without id field"(no quotes aroundid)
Additionally, the TypeScript test asserts rejects.toThrow('without id field') (no quotes), while the Python test asserts match="without 'id' field" (with quotes). This creates cross-language inconsistency that could confuse users debugging errors in polyglot environments.
Code:
# Python gasfree.py
raise RuntimeError("GasFree submit API returned data without 'id' field")// TypeScript gasfree.ts
throw new Error('GasFree submit API returned data without id field');Recommendation:
Standardise the error message across both SDKs. Prefer the quoted form ('id' field) as it is more explicit and self-documenting.
[MINOR-05] package-lock.json Version Inconsistency: Root 0.5.8 but Workspace Previously at 0.5.5
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Config |
| File | typescript/package-lock.json : Lines 1-12 |
Description:
The package-lock.json diff shows the root x402-typescript version bumped from 0.5.1 to 0.5.8, but the workspace packages/x402 version bumped from 0.5.5 to 0.5.8. The root and workspace had different version numbers to begin with (0.5.1 vs 0.5.5), which suggests the lock file root version was out of date before this PR. While this does not affect runtime behaviour (the lock file root version is informational only), it indicates the lock file was not being kept synchronised with package.json.
Code:
// package-lock.json before PR
{
"name": "x402-typescript",
"version": "0.5.1", // stale vs workspace 0.5.5
...
"packages/x402": {
"version": "0.5.5"
}
}Recommendation:
Ensure npm install is run after each version bump so the lock file root stays in sync with package.json. This can be enforced via a CI check (npm ci will fail if the lock is not up to date).
[SUGGESTION-01] No Observability for Proxy Routing: No Log When Using BankOfAI Proxy
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Documentation & Maintainability |
| File | python/x402/src/bankofai/x402/utils/gasfree.py, typescript/packages/x402/src/utils/gasfree.ts |
Description:
The previous release (v0.5.7) added an explicit logger.warning / console.warn when the fallback RPC endpoint was activated, giving operators visibility into routing decisions. This PR silently routes all GasFree API calls through the proxy with no log output. Operators upgrading from a self-configured environment (with API keys) have no in-process signal that the proxy path is in use.
Recommendation:
Add a logger.info / console.info message on first instantiation of GasFreeAPIClient (or per network, using a module-level Set) indicating the proxy URL in use:
logger.info(f"[x402] GasFree API calls for {network} routed through proxy: {self.base_url}")[SUGGESTION-02] uv.lock Churn: 2910 Line Change for Non-Dependency Update
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Code Quality |
| File | python/x402/uv.lock |
Description:
The uv.lock file changed by ~2910 lines, adding upload-time metadata fields to every wheel entry. This appears to be caused by a uv tool version upgrade rather than any actual dependency change. While harmless, it creates significant noise in diffs and makes it harder to audit genuine dependency changes.
Recommendation:
If the uv version upgrade was intentional, call it out explicitly in the commit message or PR description. Consider running uv lock --no-upgrade before committing to avoid unintended lock file regeneration. Pin the uv version in CI to prevent drift.
[SUGGESTION-03] No Migration Guide for Existing API Key Users
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Documentation & Maintainability |
| File | RELEASE_NOTES.md, CHANGELOG.md |
Description:
The PR removes GASFREE_API_KEY and GASFREE_API_SECRET environment variables as a breaking change. While this is documented in the RELEASE_NOTES, there is no explicit migration guide explaining what users should do with existing credentials (e.g., remove from .env, update CI secrets). Users who still set these environment variables will not receive a warning that they are now ignored — the variables are silently unused.
Recommendation:
- Add a "Migration Guide" section to
RELEASE_NOTES.mdwith explicit steps. - Consider adding a startup-time warning if
GASFREE_API_KEYorGASFREE_API_SECRETare still present in the environment, alerting users that these are no longer used.
4. Positive Observations
-
Clean removal of HMAC complexity: The old HMAC-SHA256 implementation involved brittle path-prefix heuristics (detecting network from URL substrings like
"open-test","nile"). Removing this entirely simplifies theGasFreeAPIClientsubstantially and eliminates a class of hard-to-debug authentication failures. -
Consistent fail-fast behaviour: Replacing the silent default fallback URL (
https://api.gasfree.io/v1) with an explicitUnsupportedNetworkErroris a correctness improvement. Silent fallbacks to wrong URLs are harder to debug than clear errors. -
Good test coverage for polling logic: The test suite has thorough coverage of
waitForSuccessedge cases — null status data, transient errors, consecutive error limits, and error-count reset after a successful poll. This coverage was preserved and expanded in the PR. -
Symmetric implementation across Python and TypeScript: The feature is implemented consistently in both SDKs, using the same API paths, the same URL scheme, and the same error message patterns. This is good for maintainability.
-
No extraneous removed code left behind: A clean search confirms no orphaned references to
getGasFreeApiKey,getGasFreeApiSecret,get_gasfree_api_key, orget_gasfree_api_secretremain in the codebase after the removal — the refactor is complete. -
Changelog and release notes are accurate and present: Both
CHANGELOG.mdandRELEASE_NOTES.mdcorrectly document the changes, version bump, breaking changes, and affected SDKs. -
requestIdadds entropy via timestamp: TherequestIdfield usesDate.now()/int(time.time())combined with a signature prefix, which provides reasonable uniqueness for replay-detection purposes.
5. Checklist Results
Correctness & Logic
| Check | Result | Notes |
|---|---|---|
| No logic errors in changed code paths | PARTIAL | requestId bug in TS submit() (MAJOR-01) |
| Null/undefined handled | PASS | Null data checks preserved for all API responses |
| Error handling complete | PASS | Errors propagated correctly |
| No resource leaks | PASS | httpx.AsyncClient used as context manager |
| API contracts maintained | PASS | Same REST endpoints, same payload structure |
Security
| Check | Result | Notes |
|---|---|---|
| No hardcoded credentials | PASS | Auth keys removed, not replaced with new hardcoded values |
| Input validation on user address | PARTIAL | User address is interpolated into URL path without URL-encoding (pre-existing) |
| No sensitive data in logs | PASS | No keys or secrets logged |
| Dependency risk | INFO | Proxy introduces a supply-chain trust dependency on BankOfAI infrastructure |
| Injection prevention | INFO | User/traceId inserted into URL path; caller-controlled but trusted via signer (pre-existing) |
Performance
| Check | Result | Notes |
|---|---|---|
| No N+1 queries | PASS | Single API call per operation |
| HTTP timeouts configured | FAIL | No per-request HTTP timeouts (MAJOR-02) |
| No unbounded operations | PASS | Polling has timeout and max-error bounds |
Code Quality
| Check | Result | Notes |
|---|---|---|
| Naming conventions consistent | PASS | |
| No dead code | PARTIAL | domain param in submit() unused (MINOR-03) |
| Type safety | PARTIAL | any types in TS submit() (MINOR-02) |
| Duplication | PASS | DRY between methods |
| Separation of concerns | PASS | Config cleanly separated from client logic |
Testing
| Check | Result | Notes |
|---|---|---|
| Changed behaviour has tests | PARTIAL | New URL values and UnsupportedNetworkError not directly tested (MINOR-01) |
| Removed behaviour tests cleaned up | PASS | HMAC tests removed correctly |
| Edge cases covered | PASS | Polling edge cases well covered |
| Test quality | PASS | Tests are clear and focused |
Documentation & Maintainability
| Check | Result | Notes |
|---|---|---|
| CHANGELOG updated | PASS | |
| RELEASE_NOTES updated | PASS | |
| Breaking changes documented | PASS | Documented in RELEASE_NOTES |
| Migration guide provided | PARTIAL | No step-by-step migration guide (SUGGESTION-03) |
| Inline code docs updated | PASS | JSDoc and Python docstrings updated |
6. Review Verdict
Decision: Request Changes
Rationale
The PR achieves its stated goal cleanly: the HMAC authentication code is fully removed, the BankOfAI proxy URLs are consistent across Python and TypeScript, and the public API change is documented. The code is simpler and more maintainable after this change.
However, two Major issues prevent approval as-is:
-
MAJOR-01 — The TypeScript
submit()method has a logic bug whererequestIdunconditionally slices the originalsignatureat position 2–10 even when the signature does not start with0x. This produces an inconsistentrequestIdrelative tosig. The fix is a one-line change. -
MAJOR-02 — Neither the Python
httpx.AsyncClientcalls nor the TypeScriptfetchcalls configure per-request HTTP timeouts. With the proxy as the sole routing path, a hung proxy connection will block the calling coroutine indefinitely during payment flows. -
MAJOR-03 — The proxy is a new single point of failure with no operator override or fallback. This is an architectural concern that should at minimum be addressed with environment-variable override support before mainnet deployment.
Required before merge:
- Fix MAJOR-01 (requestId inconsistency in TypeScript)
- Address MAJOR-02 (add HTTP request timeouts)
- Address or formally accept MAJOR-03 (document proxy SPOF risk, add env override)
- Add tests for new
UnsupportedNetworkErrorbehaviour (MINOR-01)
Can be addressed in follow-up:
- MINOR-02 through MINOR-05 and all SUGGESTION items
- Use shared httpx.AsyncClient with connection pooling instead of creating a new client per request (Python) - Add 30s timeout to all HTTP requests (both TypeScript and Python) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use shared httpx.AsyncClient with connection pooling instead of creating a new client per request (Python) - Add 30s timeout to all HTTP requests (both TypeScript and Python) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code Audit Report — PR:
|
| Property | Value |
|---|---|
| Base | remotes/origin/main |
| Head | remotes/origin/feature/bankofai-gasfree-proxy |
| Commits | 5 |
Commit Summary
61ab9e8 fix: improve GasFree API client reliability
1edac79 fix: improve GasFree API client reliability
35d5b5d refactor: remove deprecation stubs, clean break for gasfree auth removal
0e10f0e chore: bump SDK versions to 0.5.8, add deprecation stubs and changelog
a9cf5bc feat: switch gasfree API to BankOfAI proxy, remove API key/secret auth
Files Changed (15 files)
| File | Change Type |
|---|---|
CHANGELOG.md |
Modified — new v0.5.8 entry |
RELEASE_NOTES.md |
Modified — updated to v0.5.8 content |
python/x402/pyproject.toml |
Modified — version bump 0.5.7 → 0.5.8 |
python/x402/src/bankofai/x402/__init__.py |
Modified — version bump |
python/x402/src/bankofai/x402/config.py |
Modified — URLs changed, API key/secret methods removed |
python/x402/src/bankofai/x402/utils/gasfree.py |
Modified — HMAC auth removed, persistent HTTP client introduced |
python/x402/tests/utils/test_gasfree_utils.py |
Modified — signature/auth tests removed, new tests added |
python/x402/uv.lock |
Modified — lockfile refresh (revision bump) |
typescript/package-lock.json |
Modified — version bump |
typescript/package.json |
Modified — version bump 0.5.7 → 0.5.8 |
typescript/packages/x402/package.json |
Modified — version bump |
typescript/packages/x402/src/config.ts |
Modified — URLs changed, API key/secret functions removed |
typescript/packages/x402/src/mechanisms/exactGasfree.ts |
Modified — 1-line cleanup |
typescript/packages/x402/src/utils/gasfree.test.ts |
Modified — auth tests removed, new tests added |
typescript/packages/x402/src/utils/gasfree.ts |
Modified — HMAC auth removed, persistent HTTP client introduced |
Statistics
- +1,612 / -1,805 lines (net -193), dominated by the
uv.lockrefresh (+upload-time metadata) - Functional diff is much smaller: ~150 effective source-code lines changed
2. Change Summary
Group A — Proxy URL Migration (Breaking)
Both Python (config.py) and TypeScript (config.ts) GASFREE_API_BASE_URLS maps now point to https://facilitator.bankofai.io/{mainnet,shasta,nile} instead of the upstream open.gasfree.io / open-test.gasfree.io endpoints. The get_gasfree_api_base_url() / getGasFreeApiBaseUrl() helpers now throw UnsupportedNetworkError for unknown networks instead of returning a silent fallback.
Group B — HMAC Auth Removal (Breaking)
The _generate_signature() / _get_headers(method, path) methods and their HMAC-SHA256 signing logic have been fully removed from both Python and TypeScript GasFreeAPIClient. get_gasfree_api_key() / get_gasfree_api_secret() config helpers are also removed. The proxy now handles authentication transparently.
Group C — Persistent HTTP Client (Python)
Python GasFreeAPIClient.__init__ now creates a long-lived httpx.AsyncClient(timeout=30.0) instance stored as self._client, replacing the previous per-call async with httpx.AsyncClient() as client: pattern. TypeScript already used the global fetch() API.
Group D — Test Suite Updates
Auth/signature unit tests removed. New tests added covering: Authorization/Timestamp headers absent, null-data guards, missing id guard on submit, waitForSuccess polling behaviour (null status, transient errors, max-errors abort, error-counter reset).
Group E — Version Bumps and Documentation
Both SDK versions bumped from 0.5.7 to 0.5.8. CHANGELOG.md and RELEASE_NOTES.md updated accordingly. uv.lock refreshed (adds upload-time field to wheel entries, no package version changes).
3. Detailed Findings
Critical
(No Critical findings)
Major
[M-01] Resource Leak: Python httpx.AsyncClient Has No Cleanup Path
| Property | Value |
|---|---|
| Severity | Major |
| Category | Correctness & Logic / Resource Leak |
| File | python/x402/src/bankofai/x402/utils/gasfree.py : Lines 38–40 |
Description:
GasFreeAPIClient now holds a persistent httpx.AsyncClient (self._client) that is never explicitly closed. httpx.AsyncClient maintains a connection pool and should be closed (via await client.aclose() or async with) when the owning object is done. Without cleanup, TCP connections remain open until the process exits or the OS reclaims them. In long-running server processes (FastAPI, etc.) with multiple GasFreeAPIClient instances, this will exhaust available file descriptors or connection-pool slots over time.
Code:
def __init__(self, base_url: str):
self.base_url = base_url.rstrip("/")
self._client = httpx.AsyncClient(timeout=30.0)There is no __del__, async def close(), or __aenter__/__aexit__ implemented.
Recommendation:
Add an async close() / aclose() method and optionally implement __aenter__/__aexit__ to support the async with pattern. Callers (mechanism classes) should call await client.aclose() on teardown. Example:
async def aclose(self) -> None:
await self._client.aclose()
async def __aenter__(self):
return self
async def __aexit__(self, *args):
await self.aclose()[M-02] Test Mocking Fragility: Patching httpx.AsyncClient.get on a Persistent Instance
| Property | Value |
|---|---|
| Severity | Major |
| Category | Testing |
| File | python/x402/tests/utils/test_gasfree_utils.py : Lines 28–34, 54–60, 137–144, 151–158, 177–184, 203–210 |
Description:
The tests use with patch("httpx.AsyncClient.get") as mock_get: and with patch("httpx.AsyncClient.post") as mock_post:. When the client used a fresh async with httpx.AsyncClient() per call, this worked because each patched call was a fresh instance. Now that GasFreeAPIClient.__init__ creates self._client = httpx.AsyncClient(timeout=30.0) before the patch context is entered, the already-instantiated self._client is not covered by the class-level patch.
The tests appear to pass only because patch("httpx.AsyncClient.get") patches the unbound method on the class, and Python's method dispatch does use the patched version at call time. However, this is fragile: if httpx changes its internals or uses __slots__, the patch may silently stop working. The correct approach for a persistent client instance is to patch the instance method or the instance itself.
Code:
client = GasFreeAPIClient("https://api.example.com")
# _client is already constructed here ↑
with patch("httpx.AsyncClient.get") as mock_get: # fragile: patches class, not instance
mock_get.return_value = AsyncMock(...)
info = await client.get_address_info("0x123")Recommendation:
Patch the instance directly to be explicit and correct:
client = GasFreeAPIClient("https://api.example.com")
with patch.object(client._client, "get", new_callable=AsyncMock) as mock_get:
mock_get.return_value = AsyncMock(status_code=200, json=lambda: mock_response, ...)
info = await client.get_address_info("0x123")[M-03] Single-Vendor Infrastructure Dependency Without Fallback
| Property | Value |
|---|---|
| Severity | Major |
| Category | Correctness & Logic / Architecture |
| File | python/x402/src/bankofai/x402/config.py : Lines 53–57; typescript/packages/x402/src/config.ts : Lines 58–62 |
Description:
All three GasFree API networks (tron:mainnet, tron:shasta, tron:nile) now exclusively route through a single third-party proxy domain (https://facilitator.bankofai.io). The previous implementation used the upstream open.gasfree.io / open-test.gasfree.io endpoints directly. If the BankOfAI proxy experiences downtime, rate-limiting, misconfiguration, or a TLS certificate issue, every GasFree payment in all environments — including production mainnet — will fail with no fallback option.
The PR removes the env-var-based key/secret mechanism that allowed operators to override or rotate credentials. While the intent is to simplify setup, the result is that the proxy is now a single point of failure with no opt-out path.
Recommendation:
Consider one or more of: (1) preserving an optional override env var (e.g. GASFREE_API_BASE_URL_MAINNET) allowing operators to point at the upstream endpoint if the proxy is unavailable; (2) documenting the proxy's SLA and the procedure for falling back; (3) adding a simple health-check with fallback at the GasFreeAPIClient level.
Minor
[m-01] get_nonce Signature Mismatch: Unused Parameters
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Code Quality |
| File | python/x402/src/bankofai/x402/utils/gasfree.py : Lines 93–100 |
Description:
The Python get_nonce(self, user, token, chain_id) method accepts token and chain_id parameters but never uses them — the implementation simply calls self.get_address_info(user). The TypeScript equivalent getNonce(user: string) has already dropped these parameters. This inconsistency can confuse callers and suggests the Python signature was not updated during the refactor.
Code:
async def get_nonce(self, user: str, token: str, chain_id: int) -> int:
"""Get current recommended nonce for a user"""
try:
data = await self.get_address_info(user)
return data.get("nonce", 0)
except Exception as e:
...Recommendation:
Align with the TypeScript signature: async def get_nonce(self, user: str) -> int:. Any callers passing token / chain_id should be updated.
[m-02] requestId Uses sig[:8] Which May Produce Short or Predictable Identifiers
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Correctness & Logic |
| File | python/x402/src/bankofai/x402/utils/gasfree.py : Line 199; typescript/packages/x402/src/utils/gasfree.ts : Line 253 |
Description:
The requestId field is constructed as x402-{timestamp}-{sig[:8]} (Python) and x402-${Date.now()}-${signature.slice(2, 10)} (TypeScript). The timestamp component has 1-second granularity (Python uses int(time.time())) which could produce collisions if two transactions are submitted in the same second with the same first 8 bytes of signature. While the probability is low in practice, this is a correctness issue if the proxy uses requestId for idempotency.
Additionally, the Python branch strips 0x before slicing (sig = signature[2:] if signature.startswith("0x") else signature) but the TypeScript branch slices after the 0x check (signature.slice(2, 10)), meaning if the signature does NOT start with 0x in TypeScript, slice(2, 10) still skips the first two hex chars. This is a minor asymmetry.
Recommendation:
Use a UUID v4 or a cryptographically random suffix for stronger uniqueness guarantees. If the proxy requires idempotency, use a truly unique identifier rather than a timestamp+sig prefix. Align Python and TypeScript: Python uses int(time.time()) (1s resolution) while TypeScript uses Date.now() (1ms resolution) — the TypeScript version is less prone to collision.
[m-03] allowSubmit Accessed via (accountInfo as any) in TypeScript
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Code Quality / Type Safety |
| File | typescript/packages/x402/src/mechanisms/exactGasfree.ts : Lines 132–133 |
Description:
The allowSubmit field is correctly defined in the GasFreeAddressInfo interface (gasfree.ts line 31), yet the mechanism accesses it via (accountInfo as any).allowSubmit. This bypasses TypeScript's type system unnecessarily, given that accountInfo is typed as GasFreeAddressInfo. The cast was presumably added as a workaround at some point and not cleaned up.
Code:
const allowSubmit = (accountInfo as any).allowSubmit || false;Recommendation:
Remove the as any cast and use the typed field directly:
const allowSubmit = accountInfo.allowSubmit ?? false;[m-04] Missing Test Coverage for getGasFreeApiBaseUrl / get_gasfree_api_base_url Error Path
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Testing |
| File | python/x402/tests/test_config.py; TypeScript has no equivalent config test file |
Description:
The PR changes get_gasfree_api_base_url() / getGasFreeApiBaseUrl() to throw UnsupportedNetworkError for unknown networks (replacing the previous silent fallback). This is a deliberate breaking change, but there are no new tests asserting that the error is thrown for an unsupported network, nor tests asserting the correct URLs for each supported network. The existing test_config.py only tests the get_gasfree_controller_address method.
Recommendation:
Add tests such as:
def test_gasfree_api_base_url_mainnet():
assert NetworkConfig.get_gasfree_api_base_url("tron:mainnet") == \
"https://facilitator.bankofai.io/mainnet"
def test_gasfree_api_base_url_unsupported():
with pytest.raises(UnsupportedNetworkError):
NetworkConfig.get_gasfree_api_base_url("eip155:1")A parallel test for the TypeScript getGasFreeApiBaseUrl() should also be added.
[m-05] get_nonce Silently Returns 0 on Error — Potential Transaction Nonce Reuse
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Correctness & Logic |
| File | python/x402/src/bankofai/x402/utils/gasfree.py : Lines 93–100; typescript/packages/x402/src/utils/gasfree.ts : Lines 127–135 |
Description:
Both get_nonce implementations catch all exceptions and return 0 as a default. If the GasFree API is temporarily unreachable, a transaction will be submitted with nonce 0. If a previous transaction with nonce 0 is pending, the submission will fail or may be silently dropped. The silent fallback to 0 can lead to hard-to-diagnose failures.
This behaviour exists in main and is not introduced by this PR, but the refactor is a good opportunity to address it.
Recommendation:
Consider propagating the error from get_nonce rather than swallowing it, or document clearly why 0 is a safe default in this context (e.g., if the proxy guarantees idempotency for nonce 0).
Suggestions
[S-01] No TLS Certificate Pinning or Proxy Response Validation
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Security |
| File | python/x402/src/bankofai/x402/utils/gasfree.py; typescript/packages/x402/src/utils/gasfree.ts |
Description:
Traffic to facilitator.bankofai.io relies entirely on standard TLS verification. There is no additional validation of the proxy's responses beyond checking the code == 200 business field. If the proxy were ever compromised or a DNS poisoning attack succeeded, signed transaction payloads (including TRON addresses, amounts, and deadlines) would be submitted to an attacker-controlled endpoint. Given that this change centralises all three networks through a single domain this is worth noting even if TLS is the accepted industry standard.
Recommendation:
Document the trust model in the README/SECURITY.md. If the proxy is BankOfAI's own infrastructure, consider noting whether certificate pinning or a subresource integrity check is applied at the proxy layer. For production deployments, operators should ensure HTTPS_PROXY is not set in their environment.
[S-02] uv.lock Contains Newly Added upload-time Metadata — Verify Supply Chain
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Security / Dependency Risk |
| File | python/x402/uv.lock |
Description:
The uv.lock revision bumped from 2 to 3 and all wheel entries now include an upload-time field. While this appears to be a harmless uv lockfile format change, it also means every package entry was regenerated. The hashes remain the same, so no package versions changed. This is not a security issue, but a large lockfile regeneration in a feature PR is worth calling out: reviewers should verify uv lock --check passes in CI to confirm the regenerated lockfile is consistent with pyproject.toml.
Recommendation:
Confirm CI runs uv lock --check or equivalent to validate the lockfile is reproducible. Separate lockfile updates into dedicated chore commits to make reviews easier.
[S-03] domain Parameter in submit() Is Accepted But Never Used
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Code Quality / Dead Code |
| File | python/x402/src/bankofai/x402/utils/gasfree.py : Line 183; typescript/packages/x402/src/utils/gasfree.ts : Line 238 |
Description:
Both submit(domain, message, signature) methods accept a domain parameter but never reference it in the method body. The EIP-712 domain is embedded in the signature and is not required for the submission payload. The unused parameter adds noise and may confuse future maintainers into thinking the domain is being validated.
Code:
async def submit(self, domain: Dict[str, Any], message: Dict[str, Any], signature: str) -> str:
# `domain` is never used inside this methodRecommendation:
Remove the domain parameter from both implementations, update all call sites, and update the method docstring. If the domain may be needed for future validation, add a TODO comment explaining the intent.
[S-04] Logging Sensitive Address Data at INFO Level
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Security / Observability |
| File | python/x402/src/bankofai/x402/utils/gasfree.py : Line 122 |
Description:
logger.info(f"GasFree Status Response for {trace_id}: {json.dumps(data)}") logs the full status response at INFO level. The response includes accountAddress, gasFreeAddress, targetAddress, and txnHash. In a production environment with log aggregation (Datadog, Splunk, ELK), this may expose wallet addresses in log indices readable by internal teams who should not have access to user financial data.
Recommendation:
Consider logging at DEBUG level or selectively logging non-sensitive fields (e.g., state, txnState) at INFO and the full body at DEBUG.
4. Positive Observations
-
Clean removal of HMAC auth — the old
_generate_signature/_get_headers(method, path)code was non-trivial (URL prefix reconstruction, timestamp management). The PR removes it cleanly with no vestigial code paths in either language. -
Fail-fast on unsupported networks — changing
get_gasfree_api_base_urlfrom a silent default fallback to a hardUnsupportedNetworkErroris the correct defensive posture. This is documented in both CHANGELOG and RELEASE_NOTES. -
Consistent implementation across Python and TypeScript — both implementations were updated symmetrically. Interface definitions, URL maps, error handling, and tests align well between the two languages.
-
Good test coverage added — the new tests for
waitForSuccess(null data, transient error, max-errors abort, error-counter reset) are thorough and cover the important edge cases introduced in the prior v0.5.5 reliability work. -
Well-structured commit history — the 5 commits follow a logical progression (feat → chore → refactor → fix → fix) and have clear, conventional messages.
-
HTTPS enforced for all proxy URLs — all three proxy URLs use
https://, ensuring transport security for the unauthenticated relay. -
Persistent HTTP client in Python — migrating from per-call
async with httpx.AsyncClient()to a long-lived instance is a correct performance improvement for high-throughput usage; connection reuse avoids repeated TLS handshakes. -
Clean documentation — CHANGELOG entries follow the Keep a Changelog format accurately, and RELEASE_NOTES clearly calls out breaking changes.
5. Checklist Results
| Category | Status | Notes |
|---|---|---|
| Correctness & Logic | Partial | Resource leak (M-01), nonce fallback (m-05), unused params (m-01) |
| Security | Pass | HTTPS enforced; no hardcoded secrets; auth removal is intentional |
| Performance | Pass | Persistent HTTP client is an improvement |
| Code Quality | Partial | as any cast (m-03), unused domain param (S-03), sig mismatch (m-01) |
| Testing | Partial | Fragile mock strategy (M-02), missing config URL tests (m-04) |
| Documentation | Pass | CHANGELOG and RELEASE_NOTES are accurate and complete |
| Breaking Changes Documented | Pass | Clearly documented in CHANGELOG and RELEASE_NOTES |
| Version Bumps | Pass | Python pyproject.toml, __init__.py, TypeScript package.json all at 0.5.8 |
| Lock File | Pass | uv.lock updated; no package version changes |
6. Review Verdict
Request Changes
The PR achieves its stated goal cleanly: it migrates GasFree API calls through the BankOfAI proxy, removes the HMAC authentication layer, and improves SDK usability by eliminating the need for GASFREE_API_KEY / GASFREE_API_SECRET environment variables. The implementation is symmetric across Python and TypeScript, the documentation is accurate, and the test coverage additions are meaningful.
However, the following items should be addressed before merging:
Must fix (Major):
- [M-01] Add
aclose()/__aenter__/__aexit__to PythonGasFreeAPIClientto prevent connection pool exhaustion in long-running processes. - [M-02] Fix the Python test mocking strategy to use
patch.object(client._client, ...)rather than patching the class-level method, to avoid fragile tests that may silently pass with incorrect behaviour. - [M-03] The single-proxy architecture should either document the fallback procedure or preserve an opt-out env var for production operators.
Should fix (Minor):
4. [m-01] Align Python get_nonce signature with TypeScript (drop unused token and chain_id params).
5. [m-03] Remove the (accountInfo as any) cast in exactGasfree.ts.
6. [m-04] Add tests for get_gasfree_api_base_url error and success paths.
The remaining minor and suggestion items can be addressed in follow-up issues or subsequent PRs.
Prevents connection pool exhaustion in long-running processes by providing a cleanup path for the persistent httpx.AsyncClient. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code Review ReportProject: bankofai/x402 SDK PR OverviewBranch Information
Commit History
Review SummaryVerdict
Findings at a Glance
SummaryThis PR migrates GasFree API calls from the upstream The overall code quality improves significantly with this PR: authentication logic is deleted (not reimplemented), the Change Summary1. API Endpoint Migration (Both SDKs)
Purpose: Route all GasFree traffic through the BankOfAI proxy so the SDK no longer needs to manage API credentials. 2. Authentication Removal (Both SDKs)
Purpose: The proxy handles upstream HMAC auth; the client-side HMAC code is dead weight and a security maintenance liability. 3. Fail-Fast on Unknown Networks (Both SDKs)
Purpose: Prevent silent misconfiguration where an unknown network key caused requests to be silently routed to the wrong endpoint. 4. Python Client Lifecycle & Connection Pooling
Purpose: Reuse connections across API calls; enable idiomatic 5. TypeScript Per-Request Timeouts
Purpose: Prevent indefinite hangs when the proxy is slow or unresponsive. 6. Version Bumps & Documentation
7. Test Updates (Both SDKs)
Detailed FindingsMajor[MJ-01] TypeScript
|
| Property | Value |
|---|---|
| Severity | Major |
| Category | Correctness |
| File | typescript/packages/x402/src/utils/gasfree.ts : Line ~254 (submit payload) |
Description
The old TypeScript code used Math.floor(Date.now() / 1000) (Unix seconds) for the requestId timestamp, exactly matching the Python SDK's int(time.time()). The new code uses bare Date.now() (milliseconds). This introduces a cross-SDK format inconsistency in requestId values:
- Python produces:
x402-1743344862-1a2b3c4d - TypeScript now produces:
x402-1743344862000-1a2b3c4d
If the GasFree API proxy uses requestId for idempotency key matching or log correlation across SDKs, the field will now be structurally different between the two runtimes. Additionally, if any downstream consumer parses the numeric segment of requestId expecting seconds-since-epoch, it will receive a value 1000× larger than intended.
Code
// OLD (correct — matches Python)
requestId: `x402-${Math.floor(Date.now() / 1000)}-${sig.substring(0, 8)}`,
// NEW (regression — milliseconds, inconsistent with Python)
requestId: `x402-${Date.now()}-${signature.slice(2, 10)}`,Python reference (correct)
"requestId": f"x402-{int(time.time())}-{sig[:8]}",Recommendation
// Restore seconds-based timestamp AND fix the sig-slice (see MN-01):
const sigStripped = signature.startsWith('0x') ? signature.slice(2) : signature;
const payload = {
...
sig: sigStripped,
requestId: `x402-${Math.floor(Date.now() / 1000)}-${sigStripped.substring(0, 8)}`,
};Minor
[MN-01] TypeScript requestId signature slice is incorrect for non-0x-prefixed signatures
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Correctness |
| File | typescript/packages/x402/src/utils/gasfree.ts : Line ~254 (submit payload) |
Description
In submit(), the code strips the 0x prefix into sig for the sig field, but still uses the original signature variable — not sig — to build the requestId:
sig: signature.startsWith('0x') ? signature.slice(2) : signature,
requestId: `x402-${Date.now()}-${signature.slice(2, 10)}`,
// ^^^^^^^^^ uses raw `signature`, not `sig`When signature is already a bare hex string (no 0x), signature.slice(2, 10) skips the first two hex characters, producing a requestId suffix based on bytes 2–9 instead of 0–7. The Python SDK correctly uses sig[:8] (already stripped). This is inconsistent and, while subtle, can produce different requestId values for the same underlying signature depending on whether the caller includes the 0x prefix.
Code
sig: signature.startsWith('0x') ? signature.slice(2) : signature,
requestId: `x402-${Date.now()}-${signature.slice(2, 10)}`, // BUG: should use sigRecommendation
const sig = signature.startsWith('0x') ? signature.slice(2) : signature;
const payload = {
...
sig,
requestId: `x402-${Math.floor(Date.now() / 1000)}-${sig.substring(0, 8)}`,
};[MN-02] Python tests create GasFreeAPIClient without ever calling aclose()
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Testing |
| File | python/x402/tests/utils/test_gasfree_utils.py : Lines 14, 23, 38, 56, etc. |
Description
Every test in TestGasFreeAPIClient constructs GasFreeAPIClient("https://api.example.com"), which immediately opens an httpx.AsyncClient. None of the tests close the client (no aclose() call, no async with block, no autouse fixture). This triggers ResourceWarning: Unclosed <httpx.AsyncClient …> in Python 3.11+ and leaves open connections in the test process. Because the tests are synchronous setup functions calling an async object, asyncio cannot finalize the transports automatically. With strict warning filters (-W error) common in CI, this will convert a warning into a test failure.
Code
# All test methods follow this pattern — no cleanup:
def test_get_headers(self):
client = GasFreeAPIClient("https://api.example.com")
headers = client._get_headers()
...
# client._client is never closed
async def test_get_address_info(self):
client = GasFreeAPIClient("https://api.example.com")
...
# client._client is never closedRecommendation
Use the new async context manager in each test that exercises async methods, or add an autouse async fixture:
@pytest.fixture(autouse=True)
async def api_client(self):
async with GasFreeAPIClient("https://api.example.com") as client:
self.client = client
yield client
# OR inline per test:
async def test_get_address_info(self):
async with GasFreeAPIClient("https://api.example.com") as client:
with patch("httpx.AsyncClient.get") as mock_get:
...[MN-03] Python tests patch httpx.AsyncClient.get at class level, not the stored instance
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Testing |
| File | python/x402/tests/utils/test_gasfree_utils.py : Lines 27–32, 42–49 |
Description
The tests use patch("httpx.AsyncClient.get") / patch("httpx.AsyncClient.post") which replaces the method on the class, not on the specific _client instance stored in the GasFreeAPIClient. While this works (Python's MRO routes self._client.get(...) to the patched class method), it is overly broad: the patch affects any httpx.AsyncClient instantiated during the test run, including those in unrelated code. If a dependency or fixture also creates an httpx.AsyncClient, the mock would silently intercept those calls too, creating hard-to-diagnose false positives.
Code
with patch("httpx.AsyncClient.get") as mock_get: # patches the whole class
mock_get.return_value = AsyncMock(...)
info = await client.get_address_info("0x123")Recommendation
Target the specific instance attribute:
async with GasFreeAPIClient("https://api.example.com") as client:
client._client.get = AsyncMock(return_value=mock_response_obj)
info = await client.get_address_info("0x123")Suggestions
[S-01] Document the proxy trust-model change
File: CHANGELOG.md, RELEASE_NOTES.md
Description: The PR removes client-side HMAC authentication entirely. The security boundary now relies on the BankOfAI proxy (https://facilitator.bankofai.io) validating upstream credentials on behalf of all SDK users. The CHANGELOG documents what changed but not what callers should be aware of: there is no client-side evidence of authentication, no retry behavior documented for proxy outages, and no fallback to the upstream GasFree API.
Suggestion: Add a brief note in RELEASE_NOTES.md clarifying that rate limiting, auth errors, and quota enforcement are now managed at the proxy layer, and advise callers what error shapes to expect when the proxy is unavailable (e.g., 502/503 HTTP status codes).
[S-02] TypeScript GasFreeAPIClient has no cleanup API
File: typescript/packages/x402/src/utils/gasfree.ts
Description: The Python client gained aclose() + __aenter__/__aexit__ for proper resource lifecycle management. The TypeScript client uses stateless fetch() so there is nothing to clean up today. However, for API symmetry and future-proofing (e.g., if the TS client ever adopts a persistent HTTP agent or keepalive pool), an explicit no-op lifecycle method would be helpful.
Suggestion: Add a trivial async [Symbol.asyncDispose]() {} or async close() {} so consumers can write await using client = new GasFreeAPIClient(url) with TypeScript 5.2+ explicit resource management, keeping Python and TypeScript idioms aligned.
[S-03] _get_headers() in Python is an unnecessary method
File: python/x402/src/bankofai/x402/utils/gasfree.py
Description: The new _get_headers() always returns the same static dict {"Content-Type": "application/json"}. Retaining it as an instance method adds indirection with no benefit now that it carries no runtime state.
Suggestion: Replace the four call sites with a module-level constant:
_GASFREE_HEADERS: Dict[str, str] = {"Content-Type": "application/json"}or inline the literal directly. This matches the TypeScript getHeaders() which should also be considered for the same treatment if no dynamic headers are ever anticipated.
[S-04] Consider logging user address at DEBUG level only in get_address_info
File: python/x402/src/bankofai/x402/utils/gasfree.py
Description: When get_address_info fails, the error log includes the full url, which contains the user's wallet address in the path (/api/v1/address/{user}). Depending on deployment logging infrastructure, this could inadvertently expose user addresses in log aggregation systems.
Suggestion: Consider whether logging the full URL is necessary at the ERROR level, or whether f"Failed to get address info for user {user}: {e}" is sufficient context without exposing the full endpoint path.
Positive Observations
| Area | Observation |
|---|---|
| Auth simplification | Removing ~70 lines of HMAC-SHA256 authentication (key inference, path prefixing, browser/Node dual-path) eliminates an entire class of subtle signing bugs that had already surfaced multiple times. |
| Fail-fast on misconfiguration | Changing get_gasfree_api_base_url() to throw UnsupportedNetworkError instead of silently returning a wrong URL is excellent defensive practice and will surface misconfiguration at startup rather than at transaction submission time. |
| Python persistent HTTP client | Replacing async with httpx.AsyncClient() inside every method with a single pooled self._client (timeout=30s) reduces socket churn, improves throughput, and is the right architectural move. |
| TypeScript timeouts | Adding AbortSignal.timeout(30000) to every fetch() call is a meaningful reliability improvement; previously an unresponsive proxy would hang indefinitely. |
| Async context manager | Adding __aenter__ / __aexit__ / aclose() to Python GasFreeAPIClient gives callers a clean, idiomatic lifecycle API. |
| Test coverage retained | The test suite was updated in lock-step with the production code changes — no tests were simply deleted, only tests that validated now-removed HMAC behavior were replaced with equivalent tests for the new no-auth headers. |
| Changelog & release notes | Both breaking changes (function removal, fail-fast behavior) are clearly documented in CHANGELOG.md with migration guidance. |
Checklist Results
| Category | Items Checked | Pass | Fail | N/A | Notes |
|---|---|---|---|---|---|
| Correctness | 8 | 6 | 2 | 0 | requestId timestamp regression (MJ-01); sig-slice bug (MN-01) |
| Security | 6 | 5 | 0 | 1 | Proxy trust model is a deliberate design choice, not a flaw; no hardcoded secrets; no injection |
| Performance | 5 | 5 | 0 | 0 | Persistent httpx client (Python) + fetch timeouts (TS) are improvements |
| Code Quality | 8 | 7 | 0 | 1 | _get_headers() is an unnecessary indirection (S-03); submit(domain: any) loses type safety in TS |
| Testing | 5 | 2 | 3 | 0 | No aclose() after test client creation (MN-02); overly broad class-level mock (MN-03); no negative test for MJ-01 regression |
| Documentation | 4 | 3 | 0 | 1 | Proxy availability / error-shape documentation missing (S-01) |
| Compatibility | 3 | 2 | 1 | 0 | requestId format is a silent cross-SDK incompatibility (MJ-01) |
| Observability | 3 | 2 | 0 | 1 | URL in error log may expose user addresses (S-04) |
Disclaimer
This is an automated code review. It supplements but does not replace human review. The reviewer analyzed only the diff between origin/main and origin/feature/bankofai-gasfree-proxy. Runtime behavior, integration testing against the live BankOfAI proxy, and deployment impact are not covered.
Report generated by Code Review Skill v1.0.0
Date: 2026-03-30
Code Review ReportProject: bankofai-x402 (x402 Payment Protocol SDK) PR OverviewBranch Information
Commit History
Review SummaryVerdict
Findings at a Glance
SummaryThis PR migrates the GasFree API integration from direct upstream endpoints ( The overall simplification is a net positive: removing HMAC auth reduces code surface area significantly, the persistent HTTP client improves performance, and the new fail-fast However, three major issues warrant attention before merge: (1) the Python Change Summary1. API Endpoint Migration (Python
|
| Property | Value |
|---|---|
| Severity | Major |
| Category | Correctness & Logic / Resource Leaks |
| File | python/x402/src/bankofai/x402/utils/gasfree.py : Lines 37-49 |
Description:
The refactored GasFreeAPIClient now holds a persistent httpx.AsyncClient created in __init__. Async context manager support (__aenter__ / __aexit__) and aclose() were correctly added, but the two primary call-sites in the mechanisms layer (ExactGasFreeClientMechanism.__init__ in client.py and facilitator.py) store these clients in a plain dict without calling aclose() anywhere. Since Python's httpx.AsyncClient holds open connection pools, failing to close them leaks file descriptors and TCP connections, which accumulates in long-running server processes.
Neither mechanism class implements __aenter__ / __aexit__ or any teardown hook.
Code:
# python/x402/src/bankofai/x402/utils/gasfree.py line 38-40
def __init__(self, base_url: str):
self.base_url = base_url.rstrip("/")
self._client = httpx.AsyncClient(timeout=30.0)# python/x402/src/bankofai/x402/mechanisms/tron/exact_gasfree/client.py line 40-42
def __init__(self, signer: "ClientSigner", clients: Dict[str, GasFreeAPIClient]) -> None:
self._signer = signer
self._clients = clients # no lifecycle managementRecommendation:
Either (a) give ExactGasFreeClientMechanism (and its facilitator counterpart) async context-manager support that closes all held GasFreeAPIClient instances, or (b) add a close() / aclose() method and document that callers must call it. A short-term option is to document the expected lifecycle clearly in the docstring and README example.
[M-02] No test coverage for get_gasfree_api_base_url / getGasFreeApiBaseUrl throwing on unknown network (Major)
| Property | Value |
|---|---|
| Severity | Major |
| Category | Testing |
| File | python/x402/tests/utils/test_gasfree_utils.py / typescript/packages/x402/src/utils/gasfree.test.ts |
Description:
The PR introduces a breaking behavioral change: get_gasfree_api_base_url("unsupported:network") now raises UnsupportedNetworkError instead of returning a fallback URL "https://api.gasfree.io/v1". This change is documented in the CHANGELOG but has zero unit-test coverage. If the error path were accidentally reverted or altered, no test would catch it. Given that callers who pass an unrecognised network will now get an exception at construction time rather than at the first API call, this is a meaningful behavioral change that deserves explicit test coverage.
Code:
# python/x402/src/bankofai/x402/config.py lines 127-132
@classmethod
def get_gasfree_api_base_url(cls, network: str) -> str:
"""Get GasFree API Base URL for network"""
url = cls.GASFREE_API_BASE_URLS.get(network)
if not url:
raise UnsupportedNetworkError(f"No GasFree API URL configured for network: {network}")
return urlRecommendation:
Add tests in both languages:
# Python
def test_get_gasfree_api_base_url_raises_on_unknown_network():
with pytest.raises(UnsupportedNetworkError, match="No GasFree API URL configured"):
NetworkConfig.get_gasfree_api_base_url("unsupported:network")
def test_get_gasfree_api_base_url_returns_correct_urls():
assert NetworkConfig.get_gasfree_api_base_url("tron:mainnet") == \
"https://facilitator.bankofai.io/mainnet"// TypeScript (in a config.test.ts)
it('should throw UnsupportedNetworkError for unknown network', () => {
expect(() => getGasFreeApiBaseUrl('unsupported:network'))
.toThrow(UnsupportedNetworkError);
});[M-03] Inconsistency in requestId generation between Python and TypeScript submit() (Major)
| Property | Value |
|---|---|
| Severity | Major |
| Category | Correctness & Logic |
| File | python/x402/src/bankofai/x402/utils/gasfree.py : Line 208 and typescript/packages/x402/src/utils/gasfree.ts : Line 253 |
Description:
The Python implementation strips the 0x prefix from signature into sig and then uses sig[:8] (first 8 hex characters of the signature body). The TypeScript implementation uses signature.slice(2, 10) — which also takes 8 characters after 0x. These are equivalent in effect when both inputs originate from the same signing operation. However, the Python path uses int(time.time()) (seconds since epoch), while TypeScript uses Date.now() (milliseconds since epoch). This means two requestId values generated at the exact same second from Python vs. TypeScript clients will differ by three decimal digits in the timestamp component.
More importantly, if signature is shorter than 10 characters (not a realistic scenario with a well-formed signature but not validated), Python's sig[:8] silently truncates while TypeScript's signature.slice(2, 10) also silently truncates. A defensive check on minimum signature length is missing in both.
The differing timestamp precision could cause problems if the proxy deduplicates requestId values across calls within the same second from different SDKs or clients.
Code:
# Python — seconds precision
"requestId": f"x402-{int(time.time())}-{sig[:8]}",// TypeScript — milliseconds precision
requestId: `x402-${Date.now()}-${signature.slice(2, 10)}`,Recommendation:
Align the timestamp precision (use milliseconds in both, i.e. int(time.time() * 1000) in Python) and validate minimum signature length before slicing. Add a cross-SDK note in comments that the format must remain consistent:
ts_ms = int(time.time() * 1000)
"requestId": f"x402-{ts_ms}-{sig[:8]}",[N-01] _get_headers() method is now a pure constant — could be a module-level constant (Minor)
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Code Quality |
| File | python/x402/src/bankofai/x402/utils/gasfree.py : Lines 51-53 / typescript/packages/x402/src/utils/gasfree.ts : Lines 92-96 |
Description:
_get_headers() / getHeaders() now returns only {"Content-Type": "application/json"} — a static value with no inputs. The method signature exists primarily as an extension point. While this is not harmful, it creates a minor illusion of complexity (readers expect instance state or logic). Either document it as an intentional extension point for future auth headers or inline the constant directly at the call site.
Recommendation:
Add a brief comment in both implementations explaining that this is intentional:
def _get_headers(self) -> Dict[str, str]:
"""Base headers. Override or extend here if auth is needed in the future."""
return {"Content-Type": "application/json"}[N-02] Python test_get_headers accesses a private method directly (Minor)
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Testing |
| File | python/x402/tests/utils/test_gasfree_utils.py : Lines 13-18 |
Description:
The test calls client._get_headers() directly, accessing a private method (_ prefix convention). While acceptable for unit tests, this tight coupling means the test will fail if the private method is renamed or made truly private. The assertions ("Authorization" not in headers) are also somewhat redundant given the simplification — they test absence rather than the actual new behavior.
Recommendation:
Prefer testing the public observable behavior (i.e., verify that API calls succeed without auth headers by checking the fetch/httpx call arguments) rather than calling private methods directly.
[N-03] uv.lock revision bump and metadata changes included in feature branch (Minor)
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Code Quality |
| File | python/x402/uv.lock |
Description:
The uv.lock file diff is large (2910 lines in stat) because the revision was bumped from 2 to 3 and upload-time metadata was added to every package entry. The actual package dependency versions did not change. This creates review noise and makes it harder to spot genuine dependency changes. Ideally the lockfile should be regenerated cleanly before a feature PR is opened, or the lockfile update should be a separate commit clearly labelled as a chore.
Recommendation:
Consider squashing or isolating the lockfile regeneration into a dedicated commit (chore: regenerate uv.lock) to reduce diff noise in code reviews.
[N-04] RELEASE_NOTES.md overwrites v0.5.7 entry instead of prepending v0.5.8 (Minor)
| Property | Value |
|---|---|
| Severity | Minor |
| Category | Documentation & Maintainability |
| File | RELEASE_NOTES.md |
Description:
The PR replaces the entire RELEASE_NOTES.md content (which previously documented v0.5.7) with only the v0.5.8 notes. The v0.5.7 content (fallback RPC endpoint feature documentation) is lost from the file. The CHANGELOG.md preserves the full history, but RELEASE_NOTES.md no longer has a record of what v0.5.7 shipped. This is a documentation maintenance issue.
Recommendation:
Prepend the v0.5.8 section above the v0.5.7 section rather than replacing it, or clearly document in the project conventions that RELEASE_NOTES.md always reflects only the latest release.
[S-01] TypeScript: getHeaders() changed from async to sync — worth calling out in migration notes (Suggestion)
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Documentation & Maintainability |
| File | typescript/packages/x402/src/utils/gasfree.ts : Lines 92-96 |
Description:
getHeaders() was previously async (required for the HMAC crypto API calls). It is now synchronous. Any subclass or consumer that awaited the previous method would still work (awaiting a non-Promise is a no-op in JS), but if anyone overrode the method in a subclass expecting it to be async, their override would silently break. The class is not exported as an extension point in the public API surface, so this is low risk but worth a comment.
Recommendation:
No code change needed, but add a brief note in the method JSDoc indicating it is now synchronous.
[S-02] Python: missing Optional[str] return type annotation on __aexit__ (Suggestion)
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Code Quality |
| File | python/x402/src/bankofai/x402/utils/gasfree.py : Lines 45-49 |
Description:
__aenter__ and __aexit__ lack full type annotations. The *args in __aexit__ should ideally be (exc_type, exc_val, exc_tb) for clarity and type-checker support, and the return type should be Optional[bool]. This is a style/completeness issue, not a correctness issue.
Recommendation:
async def __aexit__(
self,
exc_type: type[BaseException] | None,
exc_val: BaseException | None,
exc_tb: object,
) -> None:
await self.aclose()[S-03] TypeScript submit() method still typed with domain: any, message: any (Suggestion)
| Property | Value |
|---|---|
| Severity | Suggestion |
| Category | Code Quality |
| File | typescript/packages/x402/src/utils/gasfree.ts : Line 238 |
Description:
The submit() method accepts domain: any and message: any. The domain parameter is now entirely unused in the method body (only message fields are accessed). The any types prevent TypeScript from catching field access errors on message. This pre-dates this PR but is highlighted because the removal of auth logic was an ideal opportunity to tighten the types.
Code:
async submit(domain: any, message: any, signature: string): Promise<string> {Recommendation:
Define a GasFreeSubmitMessage interface or reuse existing types for message, and either remove domain entirely or type it as Record<string, unknown> since it is not accessed.
Positive Observations
| Area | Observation |
|---|---|
| Simplification | Removing HMAC authentication significantly reduces code complexity (both languages shed ~60–80 lines). The dual-environment HMAC implementation (Node.js crypto vs. Web Crypto API) in TypeScript was fragile and is now gone. |
| Connection reuse | Python: switching from per-call httpx.AsyncClient() context managers to a single persistent client is the correct httpx usage pattern and enables HTTP/1.1 keep-alive and connection pooling. |
| Timeout discipline | TypeScript gains consistent per-request AbortSignal.timeout(30000) on all fetch() calls; Python has a 30s global timeout on the httpx.AsyncClient. Both prevent hanging requests in production. |
| Fail-fast config | Replacing the silent fallback URL with UnsupportedNetworkError is the right contract — callers with misconfigured networks will fail loudly at setup rather than at runtime during a payment. |
| Test quality | Both Python and TypeScript test suites have good coverage of happy-path, null-data, missing-id, timeout/retry, and error-reset scenarios in waitForSuccess. |
| Breaking change documentation | CHANGELOG.md clearly lists removed functions and the behavior change in getGasFreeApiBaseUrl. |
| Consistency | All three network entries (mainnet, shasta, nile) are updated atomically across both Python and TypeScript, reducing the chance of a half-migrated state. |
Checklist Results
| Category | Items Checked | Pass | Fail | N/A | Notes |
|---|---|---|---|---|---|
| A. Correctness & Logic | 6 | 4 | 2 | 0 | M-01 (resource leak), M-03 (requestId inconsistency) |
| B. Security | 5 | 5 | 0 | 0 | Auth removal is intentional and proxy-handled; no hardcoded secrets; no injection vectors |
| C. Performance | 4 | 4 | 0 | 0 | Persistent httpx client is a performance improvement; timeouts added |
| D. Code Quality | 5 | 3 | 0 | 2 | N-01 (trivial method), S-03 (any types) are suggestions only |
| E. Testing | 4 | 2 | 1 | 1 | M-02: no test for UnsupportedNetworkError on bad network in getGasFreeApiBaseUrl; N-02: private method access |
| F. Documentation & Maintainability | 4 | 2 | 0 | 0 | N-03 (lockfile noise), N-04 (RELEASE_NOTES overwrites v0.5.7) are minor issues |
Disclaimer
This is an automated code review generated by an AI assistant. While every effort has been made to identify genuine issues, the review may contain false positives or miss context-dependent behaviors. All findings should be validated by a human engineer familiar with the codebase before acting on them. The review covers only code that changed in the diff between main and feature/bankofai-gasfree-proxy; it does not constitute a full security audit of the project.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code Audit Report: feature/bankofai-gasfree-proxyDate: 2026-04-02 1. PR OverviewBranch Info
Commit Summary
Files ChangedStatistics: 15 files changed, 1625 insertions(+), 1803 deletions(-) 2. Change SummaryA. Proxy Migration (Core Feature)
B. Authentication Removal
C. Error Handling Improvement
D. Resource Management (Python)
E. Timeout Addition (TypeScript)
F. Version Bumps & Documentation
3. Detailed FindingsFINDING-01Severity: Major Description: Code snippet: // Inner: specific log + throw
console.error(`GasFree submit HTTP error ${response.status} at ${url}: ${bodyText}`);
throw new Error(`GasFree submit HTTP error: ${response.status} - Body: ${bodyText}`);
// ...
} catch (error) {
// Outer: always fires, even after inner already logged
console.error(`Failed to submit GasFree transaction: ${error}`);
throw error;
}Recommendation: FINDING-02Severity: Major Description: Code snippet: def __init__(self, base_url: str):
self.base_url = base_url.rstrip("/")
self._client = httpx.AsyncClient(timeout=30.0) # never closed unless aclose() calledRecommendation: FINDING-03Severity: Major Description: Recommendation: FINDING-04Severity: Minor Description: async submit(domain: any, message: any, signature: string): Promise<string>
Recommendation: FINDING-05Severity: Minor Description: Recommendation: FINDING-06Severity: Minor Description: Recommendation: // TypeScript
it('throws UnsupportedNetworkError for unknown network', () => {
expect(() => getGasFreeApiBaseUrl('tron:unknown')).toThrow(UnsupportedNetworkError);
});
it('returns proxy URL for tron:mainnet', () => {
expect(getGasFreeApiBaseUrl('tron:mainnet')).toBe('https://facilitator.bankofai.io/mainnet');
});# Python
def test_get_gasfree_api_base_url_unknown_raises():
with pytest.raises(UnsupportedNetworkError):
NetworkConfig.get_gasfree_api_base_url("tron:unknown")FINDING-07Severity: Suggestion Description: "requestId": f"x402-{int(time.time())}-{sig[:8]}",After stripping the Recommendation: FINDING-08Severity: Suggestion Description: def _get_headers(self) -> Dict[str, str]:
return {"Content-Type": "application/json"}The method exists to maintain the same calling convention as the old code (which had auth logic) and to make future extension easier. With auth removed, the method has no logic. This is fine as a hook for future headers (e.g., a tracing Recommendation: FINDING-09Severity: Suggestion Description: Recommendation: 4. Positive Observations
5. Checklist Results
6. Review VerdictDecision: Request ChangesRationale: The PR achieves its stated goal cleanly: the proxy migration is straightforward, the auth removal is correct, and several reliability improvements (persistent client, timeouts, fail-fast error on unknown network) are well-executed. However, two Major issues should be addressed before merge:
The remaining issues (FINDING-03 through FINDING-09) are Minor or Suggestions and can be addressed in follow-up PRs or as part of this PR at the team's discretion. |
1 participant
Summary
facilitator.bankofai.io/{mainnet,shasta,nile})GasFreeAPIClient(both TypeScript and Python)getGasFreeApiKey/getGasFreeApiSecretconfig functions and related environment variable lookupsGasFreeAPIClientconstructor to only requirebaseUrl🤖 Generated with Claude Code