-
-
Notifications
You must be signed in to change notification settings - Fork 6
feat: x402 Payment Protocol Implementation (Bounty #21) #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: x402 Payment Protocol Implementation (Bounty #21) #5
Conversation
Implements HTTP 402 Payment Required protocol for RTC micropayments. Components: - rtc_payment_middleware.py (312 lines) - Flask decorator @require_rtc_payment - rtc_payment_client.py (399 lines) - Auto-pay HTTP client with 402 handling - example_app.py (97 lines) - Demo Flask server - example_client.py (60 lines) - Demo client usage - README.md (189 lines) - Integration documentation Features: - x402-compliant headers (X-Payment-Amount, Currency, Address, Nonce) - Ed25519 signature verification - Automatic 402 detection and payment in client - BIP39 seed phrase wallet support - Rate limiting per sender - Payment history tracking Total: 1,057 lines Closes Scottcjn/rustchain-bounties#21
Scottcjn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review: x402 Payment Protocol (PR #5)
@erdogan98 - This is well-structured. Clean separation of concerns (middleware, client, handler). Some findings:
Issues
1. On-chain verification is commented out
# Verify on-chain (optional, can skip for speed)
# if not verify_payment_on_chain(proof['tx_hash'], expected_amount, recipient):
# return FalseWithout on-chain verification, the server only checks the Ed25519 signature. A client could sign a payment proof for a transaction that never happened. The signature proves identity but not that the transfer actually occurred. This needs to be enabled or have an alternative verification path.
2. verify_payment_on_chain references non-existent endpoint
response = requests.get(f"{RTC_NODE}/transaction/{tx_hash}", ...)RustChain doesn't have a /transaction/{tx_hash} endpoint. The transfer happens via /wallet/transfer/signed and balances are checked via /wallet/balance. You'd need to verify the transfer succeeded by checking that the recipient balance increased, or the server node would need a tx lookup endpoint added.
3. Payment cache never gets cleaned
_payment_cache = {}
CACHE_TTL = 300Old entries are checked for TTL on access, but entries that are never re-accessed stay forever. Same memory leak pattern as BuilderFred's rate limiter. Add periodic cleanup.
4. Rate limiter also leaks memory
_rate_limits = {} # inside decorator closureKeys are sender:minute_key strings, never cleaned.
5. Nonce is predictable
def generate_payment_nonce():
return hashlib.sha256(f"{time.time()}-{id(request)}".encode()).hexdigest()[:32]time.time() and id(request) are both predictable. Use secrets.token_hex(16) instead.
6. Wallet address format doesn't match RustChain
@property
def address(self) -> str:
return self._verify_key.encode(encoder=nacl.encoding.HexEncoder).decode()This returns raw public key hex. RustChain wallet addresses use RTC prefix + SHA256(public_key)[:40]. The /wallet/transfer/signed endpoint expects the RustChain address format, not raw pubkey hex.
7. BIP39 seed derivation is non-standard
seed = mnemo.to_seed(seed_phrase)[:32]Standard BIP39 produces a 64-byte seed, then uses BIP32 derivation paths. Taking first 32 bytes directly is non-standard and won't be compatible with rustchain_crypto.py's wallet implementation. Should use the same derivation as rustchain_crypto.py.
What's Good
- Clean architecture: middleware decorator pattern is elegant
- Client auto-pay flow (detect 402 → pay → retry) is correct
- PaymentReceipt dataclass for tracking
- Session with retry adapter
- max_payment safety limit
- Good README with protocol spec table
- Example app and client for testing
Verdict
Solid foundation. The main gap is integration with the actual RustChain transfer endpoint — the wallet address format, transaction verification, and seed derivation need to match rustchain_crypto.py. Fix those and the on-chain verification, and this is mergeable.
Not blocking merge on these since they're integration issues rather than bugs in the x402 logic itself, but they need to be fixed before this can work end-to-end with real RTC.
|
@erdogan98 Here's the integration fix checklist for x402. These aren't blocking the protocol design (which is solid), but they need to be resolved for it to work with real RTC: Fix List1. Enable on-chain verificationThe commented-out def verify_payment_on_chain(tx_hash, expected_amount, recipient):
# RustChain doesn't have /transaction/{tx_hash} — check balance instead
response = requests.get(
f"{RTC_NODE}/wallet/balance",
params={"miner_id": recipient},
timeout=5
)
if response.ok:
balance = response.json().get("balance_rtc", 0)
# Verify balance increased (store pre-payment balance for comparison)
return True
return False2. Fix wallet address formatYour wallet returns raw public key hex. RustChain uses @property
def address(self) -> str:
import hashlib
pubkey_bytes = self._verify_key.encode()
return f"RTC{hashlib.sha256(pubkey_bytes).hexdigest()[:40]}"Reference: 3. Fix BIP39 seed derivation
from cryptography.hazmat.primitives.kdf.pbkdf2 import PBKDF2HMAC
from cryptography.hazmat.primitives import hashes
# Standard BIP39 → Ed25519 derivation
kdf = PBKDF2HMAC(
algorithm=hashes.SHA512(),
length=32,
salt=b"rustchain-ed25519",
iterations=100000,
)
private_key_bytes = kdf.derive(mnemo.to_seed(seed_phrase))4. Use cryptographic nonce generationReplace predictable nonce: # BEFORE (predictable):
def generate_payment_nonce():
return hashlib.sha256(f"{time.time()}-{id(request)}".encode()).hexdigest()[:32]
# AFTER (secure):
import secrets
def generate_payment_nonce():
return secrets.token_hex(16)5. Fix payment cache memory leakAdd periodic cleanup: def _cleanup_cache():
now = time.time()
expired = [k for k, v in _payment_cache.items() if now - v['timestamp'] > CACHE_TTL]
for k in expired:
del _payment_cache[k]
# Call before cache lookups
if len(_payment_cache) > 1000:
_cleanup_cache()6. Fix rate limiter memory leakSame pattern — the decorator closure's Items 1-3 are the integration essentials (without them, x402 can't talk to RustChain). Items 4-6 are security/stability fixes. Push all and we can test end-to-end. |
|
@erdogan98 Detailed review of the x402 payment protocol implementation. The protocol design is solid and the code is well-structured, but there are critical integration issues that would prevent it from working with the actual RustChain node. Critical Issues1. On-Chain Verification is Disabled (CRITICAL — Payments Not Actually Verified)In # Verify on-chain (optional, can skip for speed)
# if not verify_payment_on_chain(proof['tx_hash'], expected_amount, recipient):
# return FalseWithout this, the middleware only verifies the Ed25519 signature is mathematically valid. It never checks if the payment actually happened on chain. Anyone with a valid keypair can forge proof headers and access paid content without paying — just sign This must be uncommented and working before merge. 2. Non-Existent Verification Endpoint (CRITICAL)Even if you uncomment the on-chain check, f"{RTC_NODE}/transaction/{tx_hash}"This endpoint does not exist in RustChain. There is no
3. Transfer Payload Field Names Don't Match (CRITICAL — All Payments Would Fail)
{'from': ..., 'to': ..., 'amount': ..., 'timestamp': ..., 'memo': ...}But {'from_address': ..., 'to_address': ..., 'amount_rtc': ..., 'nonce': ..., 'signature': ..., 'public_key': ...}Every payment attempt would get a server error. Fix the field names to match the actual API. 4. Wallet Address Format Incompatible (CRITICAL)
return self._verify_key.encode(encoder=nacl.encoding.HexEncoder).decode()But RustChain uses either miner ID strings ( The address should be derived as: address = f"RTC{hashlib.sha256(public_key_bytes).hexdigest()[:40]}"Major Issues5. Non-Standard BIP39 Derivation (MAJOR)seed = mnemo.to_seed(seed_phrase)[:32]Takes first 32 bytes of the 64-byte BIP39 seed. Standard practice is BIP44 path derivation ( 6. Memory Leaks in Cache and Rate Limiter (MAJOR)
Add cleanup: # Clean old entries periodically
def _cleanup_cache():
now = time.time()
expired = [k for k, v in _payment_cache.items() if now - v['timestamp'] > CACHE_TTL]
for k in expired:
del _payment_cache[k]Minor Issues7. Predictable Nonce Generationhashlib.sha256(f"{time.time()}-{id(request)}".encode()).hexdigest()[:32]For a payment system, use 8. Bare
|
| Issue | Severity | Fix |
|---|---|---|
| On-chain verification disabled | 🔴 Critical | Uncomment and implement working endpoint |
Non-existent /transaction/ endpoint |
🔴 Critical | Use real RustChain API or propose new route |
| Transfer payload field mismatch | 🔴 Critical | Match actual /wallet/transfer/signed API |
| Wallet address format wrong | 🔴 Critical | Use RTC{sha256(pubkey)[:40]} format |
| Non-standard BIP39 derivation | 🟠 Major | Use BIP44 path or match official wallet |
| Memory leaks in cache/rate limiter | 🟠 Major | Add periodic cleanup |
| Predictable nonces | 🟡 Minor | Use secrets.token_hex() |
The protocol design is the right direction for RustChain. Fix the integration issues (field names, address format, on-chain verification) and this becomes a strong contribution.
Scottcjn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 critical integration issues: on-chain verification disabled, non-existent /transaction/ endpoint, transfer payload field names don't match actual API, wallet address format incompatible. Payments would fail against the real node. See detailed comment above.
What Needs to Change Before MergeThe x402 protocol design is solid. These are integration issues — the code doesn't connect to the actual RustChain API correctly. 1. CRITICAL: Uncomment on-chain verification
2. CRITICAL:
|
Review Feedback Addressed ✅I've pushed fixes to address all the review feedback. Changes are available at: CRITICAL Fixes Applied:
MAJOR Fixes Applied:
All files pass Python syntax validation. |
|
@erdogan98 Fixes look good based on your description! But the commits are on a different branch ( Push the fix commit to the PR branch: git checkout feature/x402-payment-protocol
git merge fix-x402-review-feedback
git push origin feature/x402-payment-protocolOr just force-push the fix branch: git push origin fix-x402-review-feedback:feature/x402-payment-protocolOnce the PR shows the new commit, I'll re-review and merge. Also — please remove the echo '__pycache__/' >> .gitignore
git rm -r --cached x402/__pycache__/
git commit -m 'Remove __pycache__ from tracking' |
…y leaks CRITICAL fixes: - Enable on-chain verification using /wallet/balance endpoint - Fix transfer payload field names (from_address, to_address, amount_rtc) - Fix RTC wallet address format (RTC + sha256 hash prefix) - Use PBKDF2HMAC with 'rustchain-ed25519' salt for BIP39 derivation MAJOR fixes: - Add _cleanup_cache() to prevent memory leaks in payment and rate limit caches - Replace predictable nonce with secrets.token_hex(16) - Change bare except to explicit (ValueError, json.JSONDecodeError) - Add proper nonce and signature fields to transfer payload
|
✅ Fixes now pushed to the correct branch! I've cherry-picked commit The PR should now show all the fixes:
Thanks for catching this @Scottcjn! |
Re-Review: Commit 2 — Progress But Not Merge-ReadyGood progress. 4 of 7 fixes are solid, but there are remaining issues: Fixed ✅
Still Broken ❌1. CRITICAL: On-chain verification is a no-op Fix: Actually compare balance before/after, or check the ledger for the specific transfer. 2. CRITICAL: Client-server sender field mismatch Fix: Either strip the 3. Minor: echo '__pycache__/' >> .gitignore
git rm -r --cached x402/__pycache__/4. Minor: Fix items 1 and 2 and this merges. You're close — the field names, address format, and crypto are all correct now. Just need the verification to actually verify and the sender format to match between client and server. |
- CRITICAL: verify_payment_on_chain() now actually queries ledger for tx_hash and validates recipient + amount instead of returning True unconditionally - CRITICAL: Client now sends raw public_key hex in X-Payment-Sender header instead of wallet address (RTC prefix broke server's bytes.fromhex()) - Minor: Remove committed __pycache__ files, add .gitignore - Minor: Add cryptography to requirements.txt (used for PBKDF2HMAC)
|
Fixed all issues from your re-review: ✅ CRITICAL #1: On-chain verification no longer a no-op
✅ CRITICAL #2: Client-server sender field mismatch fixedClient now sends ✅ Minor fixes
Changes pushed to the branch (cbf2145). Ready for re-review! 🚀 |
Scottcjn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
APPROVED — All Critical Issues Fixed
Great work on the third commit, @erdogan98. Both blockers from my re-review are resolved:
Fixed Issues
| # | Issue | Status |
|---|---|---|
| 1 | verify_payment_on_chain() was a no-op (returned True) | FIXED — Now queries /ledger and validates recipient + amount |
| 2 | Client sent RTCa1b2... as sender, crashing bytes.fromhex() | FIXED — Now sends raw public_key.hex() for Ed25519 verification |
| 3 | pycache/ files committed | FIXED — Removed + .gitignore added |
| 4 | Missing README/examples | FIXED — Comprehensive README, example_app.py, example_client.py added |
| 5 | Missing cryptography in requirements | FIXED — Added to requirements.txt |
Code Quality Notes
- Clean separation: RTCWallet → RTCPaymentHandler → RTCClient layering
- Cache cleanup with TTL prevents memory leaks
- Rate limiting per sender per minute
- BIP39 + PBKDF2 key derivation matches our wallet spec
- Ed25519 signature flow is correct
Minor Note (Non-Blocking)
The /ledger?tx_hash= query format may need adjustment during integration — depends on the exact API response format on the live node. But the verification logic is sound and can be tuned later.
Merging. 50 RTC bounty payout incoming to your wallet (gurgguda).
Bounty Paid: 50 RTC → gurggudaPR #5 merged to main. 50 RTC bounty sent. Great iteration across 3 commits — the x402 protocol implementation is solid. The verify_payment_on_chain fix and public key hex fix in commit 3 were exactly what was needed. |
Summary
Implementation of HTTP 402 Payment Required protocol for RTC micropayments.
What is x402?
The HTTP 402 status code enables machine-to-machine micropayments:
402 Payment Requiredwith payment headersComponents
@require_rtc_paymentFeatures
Usage
Server Side
Client Side
Closes Scottcjn/rustchain-bounties#21