fix(rpc): refuse to pay Platform addresses before the v24 hard fork

[PastaPastaPasta]

Issue being fixed or feature implemented

Follow-up to #7294. Built on top of that PR's branch; only the last commit is new here — please review just fix(rpc): refuse to pay Platform addresses before the v24 hard fork.

sendtoaddress and sendmany build a version 2 asset lock for Platform recipients unconditionally, without checking whether the v24 hard fork has activated. On a node running with -acceptnonstdtxn=1 — currently the only configuration where Platform sends relay at all, since v2 asset locks are deliberately non-standard — paying a Platform address before v24 activation builds, signs and commits a consensus-invalid transaction (bad-assetlocktx-version-2).

That is not a recoverable error today. CWallet::CommitTransaction returns void and only logs the broadcast failure, so the RPC still returns a txid and the user is left with inputs marked spent, a wallet entry pending forever, a failed rebroadcast every 1-3 hours, and abandontransaction as the only way out. On default-policy nodes the standardness pre-flight happens to block it, but with an error suggesting -acceptnonstdtxn=1, which pre-fork would only make things worse.

What was done?

Gated it the same way ProTx RPCs already gate on DIP0003 in SignAndSendSpecialTx() (src/rpc/evo.cpp:434) — in the RPC layer, not in the wallet, which has never consulted deployment state.

• The check lives in ParseRecipients, next to the other Platform-address validation, and throws RPC_INVALID_PARAMETER with wording matching the existing "... requires <version>" refusals in src/rpc/evo_util.cpp:97 and src/rpc/evo.cpp:847.
• Chain::isV24Active() is added because src/wallet/rpc/ has no direct ChainstateManager access — wallet RPCs reach the node only through interfaces::Chain.

ParseRecipients is the only place in the tree that sets CRecipient::fPlatformTransfer; every other CRecipient construction uses 3-element aggregate initialisation, and the GUI cannot reach the path at all (WalletModel::validateAddress uses IsValidDestinationString, which rejects DIP-18 addresses). So the RPC-layer check covers every reachable path today. Worth noting for the Platform GUI work: a future walletmodel.cpp path that constructs Platform recipients would reach CreateTransaction without passing through ParseRecipients and would need the same guard.

How Has This Been Tested?

Extended feature_asset_locks.py: in the pre-v24 section, both sendtoaddress and sendmany to a Platform address on the permissive (-acceptnonstdtxn=1) node must now fail with -8 instead of committing an unconfirmable transaction. The full file was run end-to-end against a debug build with -DDEBUG_LOCKORDER and passes, with no lock-order warnings from the new cs_wallet -> cs_main call (the same order already used by chain().havePruned() at src/wallet/rpc/backup.cpp:143). Existing post-fork wallet tests in the same file cover that the gate does not block sends after activation.

Breaking Changes

None beyond #7294 itself. Paying a Platform address pre-v24 now fails cleanly with -8 instead of producing a stuck transaction.

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[knst]

concept NACK, because it should not be limited by wallet.

Maybe it could be soft guard in RPC code but definitely not CreateTransactionInternal.

Anyway, if there's a node is out of sync and think that 'v24' is not activated, user should still be able to create asset v2 transaction and spend funds. I think it is not typical but not rare situation. I believe, user should be warned about it but not strictly limited to create v2 asset lock prior v24 activation.

[PastaPastaPasta]

Thanks — I went looking for what the codebase already does here, and you're right about the layer. Reworking the PR accordingly. Details, since two of the three points cut differently:

Layer: agreed, this belongs in RPC. The wallet has never gated on deployment state — there is no DeploymentActive* / VersionBits / Consensus::DEPLOYMENT_* anywhere under src/wallet/, and interfaces::Chain exposes no deployment accessor at all. The established shape is SignAndSendSpecialTx() in src/rpc/evo.cpp:434, which checks DeploymentActiveAfter(tip, ..., DEPLOYMENT_DIP0003) and throws before submitting — added in 4d38776 for exactly this failure mode on ProTx. So I'll move the check to ParseRecipients, next to the other Platform-address validation, and throw RPC_INVALID_PARAMETER to match the "... requires <version>" phrasing used at src/rpc/evo_util.cpp:97 and src/rpc/evo.cpp:847.

One caveat worth flagging: src/wallet/rpc/ has no direct ChainstateManager access — wallet RPCs reach the node exclusively through interfaces::Chain. So the accessor still has to exist; only the call site moves out of CreateTransactionInternal.

Hard error vs. warning: I'd keep it an error. CWallet::CommitTransaction returns void (src/wallet/wallet.cpp:2368) and a mempool rejection is logged and dropped at line 2409, under an upstream TODO saying this is wrong for permanent failures. There is no channel to carry a warning to the caller — SendMoney, the send RPC and the GUI all report success and hand back a txid. The user is then left with inputs marked spent, a wallet entry pending forever, a failed rebroadcast every 1-3 hours, and abandontransaction as the only way out. The precedent also runs this way: bitcoin#14890 (here as 67857bc) changed createrawtransaction to throw rather than build a non-standard tx, and no send/create RPC in the tree returns a txid plus a "will not relay" warning — testmempoolaccept is the designated non-throwing probe.

On the out-of-sync node: I don't think that user can spend either way. AcceptToMemoryPool runs the same consensus check against the same index — src/validation.cpp:991 calls CheckSpecialTx(tx, m_active_chainstate.m_chain.Tip(), ...) — so a node that believes v24 is inactive rejects its own v2 asset lock with bad-assetlocktx-version-2. Dropping the guard doesn't unblock them; it moves the failure to after the transaction is already committed to the wallet.

A question for your PR rather than mine. There's a third pattern in the tree that would serve the out-of-sync user properly: DeploymentToProtxVersion() (src/validation.cpp:5838) derives the payload version from deployment state — including from DEPLOYMENT_V24 — and the RPC then refuses only the parameters the resulting version cannot carry. Applied to asset locks that would mean picking the version from v24 state instead of hardcoding CURRENT_VERSION at src/wallet/spend.cpp:809, and refusing only a P2SH Platform destination pre-v24 — a P2PKH Platform address is expressible as a v1 asset lock, which is standard, relayable, and processable by Platform today, whereas v2 is deliberately non-standard until Platform can handle it.

I left that out of this PR because it changes your feature's semantics rather than just its guard rails, and there's one thing I can't answer from the code: does Platform interpret the credit output differently based on the payload version, or only on the output script? If it's only the script, then a v1 lock to a DIP-18 P2PKH address is indistinguishable on-chain and users could send to Platform addresses before v24 and on default-policy nodes. Happy to implement it if you think that holds.

---

🤖 Posted autonomously by Claude on behalf of pasta.

[PastaPastaPasta]

Reworked in c9fa309 — the check moved out of CreateTransactionInternal into ParseRecipients, throwing RPC_INVALID_PARAMETER with wording matching src/rpc/evo_util.cpp:97. PR title and description updated to match.

Verified: full feature_asset_locks.py passes against a debug build with -DDEBUG_LOCKORDER, no lock-order warnings from the new cs_wallet -> cs_main call (same order as the existing chain().havePruned() call under cs_wallet at src/wallet/rpc/backup.cpp:143).

One consequence worth naming: ParseRecipients is currently the only place that sets CRecipient::fPlatformTransfer, so coverage is unchanged today — but CreateTransactionInternal was the funnel every caller shares, and it will now build a v2 payload for any recipient carrying that flag. The Platform GUI work adds a walletmodel.cpp path that constructs recipients directly and would bypass this, so that path will need the same guard (or should route through shared validation).

The version-derivation question from my previous comment still stands if you'd rather solve it at the root.

---

🤖 Posted autonomously by Claude on behalf of pasta.

[PastaPastaPasta]

LGTM

[thepastaclaw]

✅ Final review complete — no blockers (commit c9fa309)

[coderabbitai]

Walkthrough

The public chain interface now exposes v24 activation status. NodeImpl checks the V24 deployment at the active chain tip while holding cs_main. Wallet sendtoaddress and sendmany pass the chain interface to ParseRecipients, which rejects Platform-address payments before v24 activation. Functional tests cover both RPC methods.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score: ⚪ Minimal · up to c9fa3

The change cleanly rejects Platform payments before v24 and preserves the post-v24 path. No actionable merge-blocking risk remains; adding explicit post-v24 sendmany coverage is a minor follow-up for test completeness.

Sequence Diagram(s)

sequenceDiagram
  participant RPCClient
  participant WalletRPC
  participant ParseRecipients
  participant Chain
  RPCClient->>WalletRPC: sendtoaddress or sendmany with Platform address
  WalletRPC->>ParseRecipients: pass wallet chain interface
  ParseRecipients->>Chain: isV24Active()
  Chain-->>ParseRecipients: activation status
  ParseRecipients-->>WalletRPC: invalid-parameter error when inactive
  WalletRPC-->>RPCClient: reject payment

Loading

Possibly related PRs

• dashpay/dash#7294: Both changes update ParseRecipients to coordinate Platform-address handling with v24 asset-lock activation.

Suggested reviewers: knst

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: rejecting Platform-address payments before v24 activation.
Description check	✅ Passed	The description directly explains the pre-v24 RPC failure, implementation approach, rationale, testing, and affected commands.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/functional/feature_asset_locks.py`:
- Around line 777-782: Add post-v24 coverage alongside the existing
sendtoaddress assertions in the asset-lock functional test: invoke
node_wallet.sendmany with the Platform address after v24 activation, verify the
resulting asset-lock transaction is version 2, and update the expected
credit-pool accounting to include this additional transaction.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 57dc7200-1fc3-4d73-8c2a-936a614072c3

📥 Commits

Reviewing files that changed from the base of the PR and between 4defcfb and c9fa309.

📒 Files selected for processing (4)

• src/interfaces/chain.h
• src/node/interfaces.cpp
• src/wallet/rpc/spend.cpp
• test/functional/feature_asset_locks.py

[thepastaclaw]

Final validation — Codex/Sol only (Phase 2 disabled)

At exact head c9fa309, the v24 gate uses next-block activation semantics and both affected RPCs exercise the rejection path; the existing post-v24 sendtoaddress test exercises the active branch of their shared ParseRecipients implementation. The requested post-v24 sendmany case would duplicate that shared branch rather than cover distinct changed behavior, so no actionable issue remains. Source: reviewer backend model gpt-5.6-sol; final verifier backend model gpt-5.6-sol. openclaw-agent/cliproxy/gpt-5.6-sol is orchestration-only and not reviewer evidence.

Validated zero-blocker Codex/Sol precheck evidence was promoted to final because Phase 2 (Sonnet/Opus) is temporarily disabled. This is Codex/Sol-only final validation, not Codex + Sonnet/Opus coverage.

Review provenance

• Codex reviewers: gpt-5.6-sol — general (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet/Opus: not run (Phase 2 disabled — temporary Codex/Sol-only final)
• Secondary pass: disabled (temporary_phase2_sonnet_disable)

[knst]

lgtm c9fa309

[knst]

— so a node that believes v24 is inactive rejects its own v2 asset lock with bad-assetlocktx-version-2.

That's true, but createrawtransaction produces hex of transaction and doesn't submit it to mempool.

[thepastaclaw]

Right — createrawtransaction is different because it returns unsigned hex and never commits or submits it. This PR does not change or gate that RPC; it only guards sendtoaddress and sendmany, which immediately commit/broadcast and currently return a txid even when the local mempool permanently rejects the transaction.

So an advanced caller can still construct/export raw transaction data without this guard. The hard error here is specifically to keep the wallet send RPCs from reporting success while leaving behind a stuck wallet transaction.

---

🤖 Posted autonomously by PastaClaw on behalf of pasta.