Skip to content

Advertise MCP schemas for wallet write tools#989

Open
elianguitarra wants to merge 1 commit into
ramimbo:mainfrom
elianguitarra:codex/mcp-wallet-tool-schemas-934
Open

Advertise MCP schemas for wallet write tools#989
elianguitarra wants to merge 1 commit into
ramimbo:mainfrom
elianguitarra:codex/mcp-wallet-tool-schemas-934

Conversation

@elianguitarra
Copy link
Copy Markdown

@elianguitarra elianguitarra commented Jun 6, 2026

Summary

  • add ools/list input schemas for
    egister_wallet and submit_wallet_transfer
  • document required wallet write arguments, address/signature/public-key patterns, nonce minimums, and label/memo limits in the advertised schemas
  • extend MCP tools/list coverage so these schemas stay aligned with runtime validation

Validation

  • .\.venv\Scripts\python.exe -m pytest tests\test_api_mcp.py::test_mcp_tools_list_and_call tests\test_api_mcp.py::test_mcp_can_register_and_fetch_wallet tests\test_api_mcp.py::test_mcp_wallet_write_tools_reject_unexpected_arguments
  • .\.venv\Scripts\ruff.exe check app\mcp.py tests\test_api_mcp.py

Refs #934

Summary by CodeRabbit

  • New Features

    • Wallet registration and transfer inputs now enforce explicit validation: public keys must be 64-hex, addresses must be mrwk1-prefixed 40-hex, signatures must be 128-hex, required fields (e.g., public key, from/to, amount, nonce, signature) are enforced, memo and label length limits apply, and extraneous fields are rejected.
  • Tests

    • Added tests covering the new wallet input validation rules.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 6, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a7ad8bd5-e227-41b5-b6f6-0dcad0eb11fc

📥 Commits

Reviewing files that changed from the base of the PR and between c2b1807 and c04b014.

📒 Files selected for processing (2)
  • app/mcp.py
  • tests/test_api_mcp.py

📝 Walkthrough

Walkthrough

This PR adds explicit JSON Schema inputSchema definitions to two wallet-related MCP tools: register_wallet (public key hex validation, optional label) and submit_wallet_transfer (mrwk1 address pattern, required amount/nonce/signature, optional memo). Tests assert all schema constraints.

Changes

MCP Wallet Tool Input Schemas

Layer / File(s) Summary
Wallet tool input schemas and validation
app/mcp.py, tests/test_api_mcp.py
register_wallet validates public_key_hex (64-hex), optional label, and rejects additional properties. submit_wallet_transfer validates from_address/to_address (mrwk1-prefixed 40-hex), requires amount_mrwk, nonce (minimum 1), and signature_hex (128-hex), allows optional memo, and rejects additional properties. Tests assert required fields, patterns, lengths, minimums, and additionalProperties: False.

Possibly related issues

Possibly Related PRs

  • ramimbo/mergework#937: Modifies app/mcp.py MCP_TOOLS inputSchema definitions and updates the same test suite.
  • ramimbo/mergework#927: Tightens runtime argument validation for the same wallet MCP tools (register_wallet, submit_wallet_transfer).
  • ramimbo/mergework#926: Adds JSON inputSchema entries and tests for related wallet/get_wallet MCP tool validation.
🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Bounty Pr Focus ⚠️ Warning Commit includes 172 files; PR summary states only app/mcp.py and tests/test_api_mcp.py changed. Contains 170 unrelated files violating bounty PR focus scope. Rebase to show only stated file changes (app/mcp.py, tests/test_api_mcp.py) or clarify if this is the correct PR baseline.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Advertise MCP schemas for wallet write tools' clearly and concretely names the changed surface: adding JSON Schema definitions to the MCP tools for wallet operations.
Description check ✅ Passed The description covers the main changes, validation steps, and issue reference (#934). It documents the schema additions, required arguments, patterns, and nonce/limit constraints. All required template sections are addressed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Mergework Public Artifact Hygiene ✅ Passed No investment, price, cash-out, or payout claims found. MRWK referenced only as native coin in technical wallet schemas.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@elianguitarra
Copy link
Copy Markdown
Author

Rebased onto current origin/main to clear the dirty merge state.\n\nPost-rebase validation:\n- .venv\Scripts\python.exe -m pytest tests\test_api_mcp.py::test_mcp_tools_list_and_call tests\test_api_mcp.py::test_mcp_can_register_and_fetch_wallet tests\test_api_mcp.py::test_mcp_wallet_write_tools_reject_unexpected_arguments -> 3 passed, 1 existing Starlette/httpx warning\n- .venv\Scripts\ruff.exe check app\mcp.py tests\test_api_mcp.py -> All checks passed\n- .venv\Scripts\ruff.exe format --check app\mcp.py tests\test_api_mcp.py -> 2 files already formatted\n- git diff --check origin/main...HEAD -> clean\n\nCurrent diff remains focused on app/mcp.py and tests/test_api_mcp.py, preserving main's ledger/proof MCP schema assertions while adding wallet write tool inputSchema coverage.

@elianguitarra elianguitarra force-pushed the codex/mcp-wallet-tool-schemas-934 branch from c2b1807 to c04b014 Compare June 6, 2026 13:32
@elianguitarra
Copy link
Copy Markdown
Author

Confirmed after rebase: the current PR diff is exactly two files: app/mcp.py and tests/test_api_mcp.py. GitHub reports base 2712881, head c04b014, one commit, and those same two files. The earlier 172-file bounty focus warning is stale from the pre-rebase comparison; the current diff is scoped to the stated MCP wallet schema changes.

Copy link
Copy Markdown

@modelsbridgeaicom-ship-it modelsbridgeaicom-ship-it left a comment

Choose a reason for hiding this comment

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

Approved current head c04b014709a8924dd86e3da0053d08f90e7e8025 from the MCP wallet write-tool schema surface.

The PR is focused on adding tools/list input schemas for register_wallet and submit_wallet_transfer without changing runtime behavior. I checked the schema metadata against the existing wallet argument validation expectations, including required write fields, mrwk1 address shape, public key/signature hex constraints, nonce minimums, label/memo limits, and rejection of unexpected arguments.

Files inspected:

  • app/mcp.py
  • tests/test_api_mcp.py

Validation run locally:

  • .\.venv\Scripts\python.exe -m pytest tests\test_api_mcp.py::test_mcp_tools_list_and_call tests\test_api_mcp.py::test_mcp_can_register_and_fetch_wallet tests\test_api_mcp.py::test_mcp_wallet_write_tools_reject_unexpected_arguments -q -> 3 passed, 1 existing Starlette/httpx warning.
  • .\.venv\Scripts\python.exe -m pytest tests\test_api_mcp.py tests\test_mcp_tools.py -q -> 131 passed, 1 existing Starlette/httpx warning.
  • .\.venv\Scripts\ruff.exe check app\mcp.py tests\test_api_mcp.py -> All checks passed.
  • .\.venv\Scripts\ruff.exe format --check app\mcp.py tests\test_api_mcp.py -> 2 files already formatted.
  • .\.venv\Scripts\python.exe -m mypy app\mcp.py app\mcp_tools.py app\wallet_api.py -> success.
  • .\.venv\Scripts\python.exe scripts\docs_smoke.py -> docs smoke ok.
  • git diff --check origin/main...HEAD -> clean.
  • git merge-tree --write-tree origin/main HEAD -> clean tree 022403842bd388d52d4edc1b58ca5aa5b7cf32a4.

GitHub state checked before review: PR open, non-draft, mergeable, CodeRabbit success, no current-head human reviews, and no visible #933 claim for PR #989. The hosted standard Quality/readiness/docs/image check is not present in the current rollup, so maintainers should still trigger or wait for it if required before merge.

Scope boundaries: no runtime MCP behavior, ledger mutation, wallet signing/custody, treasury/proposal execution, payout execution, admin-token behavior, credentials, private data, exchange, bridge, cash-out, MRWK price behavior, or fabricated payout claim was used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants