Repository navigation
fix(mcp): make mainnet key optional and explicit opt-in - #554
esanbasirat wants to merge 3 commits into
Conversation
|
@esanbasirat Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Miracle656
left a comment
There was a problem hiding this comment.
The idea is right and I want it: requiring an explicit opt-in before a mainnet key is ever used, and keeping read-only operation possible with no keys at all, is the correct shape for a server whose paid_call tool spends real money. setupMcpPayment is clean, and rule 4 — never log or leak a key value — being written down beside the code is the part that survives.
It needs a rebase, because main moved under it today.
What changed beneath you
#521 landed this morning and reworked src/mcp/server.ts substantially: an authenticated Streamable HTTP transport, a spend budget with payment_budget, a payment ledger, and bounded tool results. Your branch predates all of it, so:
src/mcp/server.ts 2 conflicts, 71 lines
examples/mcp-server/run.ts 3 conflicts, 98 lines
tests/mcp.test.ts 2 conflicts, 1465 lines
The server.ts conflicts are not adjacent additions — they are two different versions of the same tool-dispatch block, one of which now has to decide what paid_call does when a budget is configured. tests/mcp.test.ts is effectively a whole-file conflict.
I resolve a lot of conflicts in these reviews, and I am not resolving this one: it is the dispatch path for a tool that moves funds, and picking between two versions of it is a decision about behaviour rather than a merge. You know which half is current; I would be guessing.
What should survive the rebase
src/mcp/paymentConfig.ts is the valuable part and barely touches what moved. Rebased, it should sit in front of #521's setup rather than beside it — the server now builds fetchWithPayment, a budget and a ledger together, so the question becomes "what does setupMcpPayment hand that pipeline when mainnet is not enabled".
Two things to check while you are in there, both from #521:
paid_callis now refused withERR_PAYMENT_REJECTEDwhenfetchWithPaymentis absent. Your "empty schemes" path should produce that same refusal rather than a fetch that tries and fails further down.- There is a
payment_budgettool reporting the spend ceiling. If mainnet is configured but not enabled, that should be visible there rather than only in a startup warning.
After rebasing, npx tsc --noEmit and npx vitest run both clean, and I will come back to it quickly. The three files it conflicts in are all ones #521 rewrote, so a rebase will be a real piece of work — but it is the only way to land this safely.
Summary
This PR addresses #412 by removing the requirement to configure a mainnet secret key when running the MCP server against testnet, making mainnet payment signing an explicit opt-in, enabling read-only startup when no keys are present, and cleanly reporting missing network signer errors without ever leaking secret keys.
What is included:
src/mcp/paymentConfig.ts:setupMcpPayment()to configure payment schemes dynamically based on available keys.enableMainnet: trueviaMCP_ENABLE_MAINNET=true,MCP_ALLOW_MAINNET=true, or--allow-mainnet); ignores mainnet key if not opted in and emits informational guidance.⚠️ WARNING: Mainnet (stellar:pubnet) is enabled! Real funds are reachable.src/mcp/server.ts:fetchWithPaymentoptional inMcpServerConfigandBazaarMcpServer.paid_callgracefully when payment is disabled or unconfigured (ERR_PAYMENT_REJECTED).handlePaidCallErrorso missing network scheme errors (No network/scheme registered for <network>) map toERR_PAYMENT_REJECTEDand explicitly name the required network.examples/mcp-server/run.ts:setupMcpPayment().examples/mcp-server/README.md:tests/mcp.test.ts:paid_callrefusing.Closes #412