From d01a6b7302b39b9f10a4f2ebcf8468bcd5e17f6f Mon Sep 17 00:00:00 2001 From: savecharlie Date: Tue, 30 Jun 2026 18:03:59 -0700 Subject: [PATCH] fix(wallet): drop `fee` from signed transfer message so signatures verify MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed transfers still failed after PR #248. The client must sign a message byte-identical to what the node reconstructs at POST /wallet/transfer/signed. The node (createkr/Rustchain) reconstructs and verifies: tx_data = {"from","to","amount","memo","nonce"} (+ "chain_id" iff sent) message = json.dumps(tx_data, sort_keys=True, separators=(",",":")) That message has NO `fee` key and signs `nonce` as a string. PR #248 included `"fee": 0.0` in the signed JSON — a key the node never signs — so the sorted-key bytes differed by the `"fee":0.0,` segment and every signature still failed verification. - Remove `fee` from the signed tx_data (client now signs exactly the node's five fields; amount is float(amount_rtc) to match the node's _safe_float). - Simplify the POST body to the legacy field names the node's preflight reads (from_address / to_address / amount_rtc + public_key); drop the redundant canonical duplicates and dead fee/fee_rtc fields. - Keep nonce signed as a string — the node does str(nonce), so this was already correct (no change). - Add tests/test_transfer_signed_message_format.py: proves the client signature verifies against the node's exact reconstruction, and that a message with a `fee` key does NOT verify (fee-free invariant). Verified with a real Ed25519 round trip (PyNaCl): the fixed client message is byte-identical to the node reconstruction and verifies True; the prior with-fee message verifies False. Co-Authored-By: Iris (Opus 4.8, 1M) --- rustchain_mcp/server.py | 43 +++--- tests/test_transfer_signed_message_format.py | 140 +++++++++++++++++++ 2 files changed, 163 insertions(+), 20 deletions(-) create mode 100644 tests/test_transfer_signed_message_format.py diff --git a/rustchain_mcp/server.py b/rustchain_mcp/server.py index 8b0a3b5..2dfe021 100644 --- a/rustchain_mcp/server.py +++ b/rustchain_mcp/server.py @@ -279,25 +279,30 @@ def wallet_transfer_signed( "hint": "Use wallet_list to see available wallets", } - # Sign the transfer using the network's CANONICAL format: compact, sorted-key - # JSON of {from,to,amount,fee,memo,nonce}, matching the official - # rustchain_sdk.Wallet.sign_transfer(). The previous colon-delimited string - # signed a different message than the node verifies — AND a *second*, separate - # nonce was generated at submit time — so every signed transfer was rejected. + # Sign EXACTLY the message the node reconstructs and verifies at + # POST /wallet/transfer/signed (createkr/Rustchain node handler): + # + # tx_data = {"from","to","amount","memo","nonce"} (+ "chain_id" iff sent) + # message = json.dumps(tx_data, sort_keys=True, separators=(",", ":")) + # + # Critical: the node's signed message has NO `fee` key, and `nonce` is a + # STRING (the node does str(nonce) before signing). The prior merged fix + # included `"fee": 0.0` in the signed JSON, which the node never signs — so + # the sorted-key bytes differed by the `"fee":0.0,` segment and every + # signature still failed verification. `amount` is the RTC value as a float, + # matching the node's _safe_float(amount_rtc). nonce = int(time.time() * 1000) - fee_rtc = 0.0 tx_data = { "from": wallet["address"], "to": to_address, "amount": float(amount_rtc), - "fee": float(fee_rtc), "memo": memo, "nonce": str(nonce), } transfer_message = json.dumps(tx_data, sort_keys=True, separators=(",", ":")).encode() signature = rustchain_crypto.sign_message(transfer_message, wallet["private_key"]) - - # Submit signed transfer to network — passing the SAME nonce/fee that were signed + + # Submit signed transfer to network — passing the SAME nonce that was signed result = rustchain_transfer_signed( from_address=wallet["address"], to_address=to_address, @@ -306,7 +311,6 @@ def wallet_transfer_signed( public_key=wallet["public_key"], memo=memo, nonce=nonce, - fee_rtc=fee_rtc, ) return { @@ -454,7 +458,6 @@ def rustchain_transfer_signed( public_key: str, memo: str = "", nonce: int = None, - fee_rtc: float = 0.0, ) -> dict: """Transfer RTC tokens between wallets (requires Ed25519 signature). @@ -472,18 +475,18 @@ def rustchain_transfer_signed( import time if nonce is None: nonce = int(time.time() * 1000) - # Send both canonical ({from,to,amount,fee}) and legacy ({from_address,...}) - # field names so the node can reconstruct the exact signed message regardless - # of which it reads. nonce/fee MUST match what the caller signed. + # Send exactly the fields the node's /wallet/transfer/signed handler reads + # (from_address, to_address, amount_rtc, nonce, signature, public_key, memo). + # `amount_rtc` is float()-normalized so it serializes identically to the + # `amount` the client signed. The node ignores fee on this endpoint (its + # signed message has no fee), so no fee field is sent. The earlier + # dual-field payload (canonical `from`/`to`/`amount` alongside legacy names) + # was redundant — the node only reads the legacy names — and the extra `fee` + # fields were dead weight. payload = { - "from": from_address, - "to": to_address, - "amount": float(amount_rtc), - "fee": float(fee_rtc), "from_address": from_address, "to_address": to_address, - "amount_rtc": amount_rtc, - "fee_rtc": float(fee_rtc), + "amount_rtc": float(amount_rtc), "memo": memo, "nonce": nonce, "signature": signature, diff --git a/tests/test_transfer_signed_message_format.py b/tests/test_transfer_signed_message_format.py new file mode 100644 index 0000000..e0e33b9 --- /dev/null +++ b/tests/test_transfer_signed_message_format.py @@ -0,0 +1,140 @@ +"""Regression tests for the /wallet/transfer/signed message format. + +These guard the exact class of bug that made signed transfers unusable: the +client must sign *byte-for-byte* the message the RustChain node reconstructs +and verifies. The node handler (createkr/Rustchain, +node/rustchain_v2_integrated_v2.2.1_rip200.py, POST /wallet/transfer/signed) +reconstructs: + + tx_data = {"from", "to", "amount", "memo", "nonce"} (+ "chain_id" iff sent) + message = json.dumps(tx_data, sort_keys=True, separators=(",", ":")) + +Key invariants verified here: + * NO `fee` key in the signed message (the node does not sign fee). + * `nonce` is signed as a STRING. + * The POST body carries the legacy field names the node reads + (from_address / to_address / amount_rtc) plus public_key — and NOT the + redundant canonical duplicates or any fee field. + * The signature produced by the client verifies against the node's exact + reconstruction. +""" +import json +import shutil +import tempfile +from pathlib import Path +from unittest import mock + +import pytest + +from rustchain_mcp import rustchain_crypto +from rustchain_mcp import server + +# The @mcp.tool() decorator wraps these as FunctionTool; .fn is the raw function. +_wallet_transfer_signed = server.wallet_transfer_signed.fn + + +@pytest.fixture +def temp_keystore(): + """Temporary keystore dir (mirrors tests/test_wallet_tools.py).""" + temp_dir = tempfile.mkdtemp() + with mock.patch.object(rustchain_crypto.Path, "home", return_value=Path(temp_dir)): + yield Path(temp_dir) / ".rustchain" / "mcp_wallets" + shutil.rmtree(temp_dir, ignore_errors=True) + + +def _node_reconstruct(payload: dict) -> bytes: + """Rebuild the exact bytes the node signs/verifies, from the POST body.""" + tx = { + "from": str(payload["from_address"]).strip(), + "to": str(payload["to_address"]).strip(), + "amount": float(payload["amount_rtc"]), + "memo": str(payload.get("memo", "")), + "nonce": str(int(str(payload["nonce"]))), + } + chain_id = str(payload.get("chain_id", "")).strip() + if chain_id: + tx["chain_id"] = chain_id + return json.dumps(tx, sort_keys=True, separators=(",", ":")).encode() + + +def _run_transfer(temp_keystore, memo="", amount_rtc=1.5): + rustchain_crypto.create_wallet("signed-fmt-sender", password="") + captured = {} + + resp = mock.Mock() + resp.json.return_value = {"transaction_id": "tx_test", "new_balance": 0.0} + resp.raise_for_status = mock.Mock() + + def fake_post(url, json=None, **kwargs): + captured["url"] = url + captured["payload"] = json + return resp + + # Under the installed fastmcp, @mcp.tool() wraps module functions as + # FunctionTool, so the internal wallet_transfer_signed -> rustchain_transfer_signed + # call must be pointed at the raw .fn to run. This still exercises the real + # payload-construction code under test. + with mock.patch("rustchain_mcp.server.get_client") as gc, \ + mock.patch("rustchain_mcp.server.rustchain_transfer_signed", + server.rustchain_transfer_signed.fn): + client = mock.Mock() + client.post.side_effect = fake_post + gc.return_value = client + _wallet_transfer_signed( + from_wallet_id="signed-fmt-sender", + to_address="RTC" + "b" * 40, + amount_rtc=amount_rtc, + password="", + memo=memo, + ) + return captured["payload"] + + +def test_signed_message_has_no_fee_and_verifies(temp_keystore): + payload = _run_transfer(temp_keystore, memo="bounty payment") + msg = _node_reconstruct(payload) + assert b'"fee"' not in msg + assert rustchain_crypto.verify_signature( + msg, payload["signature"], payload["public_key"] + ), "client signature must verify against the node's reconstructed message" + + +def test_signed_message_verifies_with_empty_memo(temp_keystore): + payload = _run_transfer(temp_keystore, memo="") + msg = _node_reconstruct(payload) + assert rustchain_crypto.verify_signature( + msg, payload["signature"], payload["public_key"] + ) + + +def test_payload_uses_node_field_names_only(temp_keystore): + payload = _run_transfer(temp_keystore, memo="x") + # Fields the node's preflight requires: + for field in ("from_address", "to_address", "amount_rtc", "nonce", + "signature", "public_key"): + assert field in payload, f"missing node-required field: {field}" + # No fee anywhere, and no redundant canonical duplicates. + assert "fee" not in payload and "fee_rtc" not in payload + assert "from" not in payload and "to" not in payload and "amount" not in payload + + +def test_nonce_is_signed_as_string(temp_keystore): + payload = _run_transfer(temp_keystore) + msg = _node_reconstruct(payload) + obj = json.loads(msg) + assert isinstance(obj["nonce"], str), "nonce must be signed as a string" + + +def test_with_fee_key_would_not_verify(temp_keystore): + """Guard: a message that includes `fee` (the old bug) must NOT verify — + proving the node-format is fee-free.""" + payload = _run_transfer(temp_keystore, memo="regression") + tx = { + "from": payload["from_address"], "to": payload["to_address"], + "amount": float(payload["amount_rtc"]), "fee": 0.0, + "memo": payload.get("memo", ""), "nonce": str(int(str(payload["nonce"]))), + } + bad_msg = json.dumps(tx, sort_keys=True, separators=(",", ":")).encode() + assert not rustchain_crypto.verify_signature( + bad_msg, payload["signature"], payload["public_key"] + ), "signature must NOT verify against a message that adds a fee field"