[knst]

Issue being fixed or feature implemented

Platform ignores asset-locks with amount of inputs more than 100 or with size of transaction bigger than 20480 bytes.
If user will create too big asset-lock transaction that platform (L2) can not recognize it may cause to fund lost.
Issue is worsened once user will have ability to create asset-lock tx himself by Dash Core after #7294

It's an addition to this PR: https://github.com/dashpay/platform/pull/3491/files

What was done?

Asset locks that have more than 100 inputs are non-standard txes
Asset locks that have size bigger than 20480 bytes are non-standard txes
Asset locks that have v2 payload are non-standard txes. Once v24 is activated, platform may be not ready to process them. This non-standard limitation is a feature for soft enabling v2 asset-locks without 2nd fork.

How Has This Been Tested?

See updates in functional test.

Breaking Changes

Some special transactions are now valid for consensus if they are mined in block but they are non-standard and won't be relayed to network.

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

[thepastaclaw]

✅ Review complete (commit c8801b2)

[github-actions]

⚠️ Potential Merge Conflicts Detected

This PR has potential conflicts with the following open PRs:

• feat: asset lock tx v2 with basic RPC support + tests #7294 - feat: asset lock tx v2 with basic RPC support + tests

Please coordinate with the authors of these PRs to avoid merge conflicts.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5f0e2fcf97

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[coderabbitai]

Walkthrough

This PR introduces standardness validation for asset lock special transactions in the Dash mempool. A new IsStandardSpecialTx function enforces three constraints on asset lock transactions: maximum 100 inputs, maximum 20,480 bytes total serialized size, and rejection of payload version >= 2. The function is integrated into MemPoolAccept::PreChecks when fRequireStandard is enabled to reject non-standard asset locks with descriptive reason codes. Functional tests and helper updates verify mempool rejection/acceptance behavior and block inclusion.

Sequence Diagram

sequenceDiagram
  participant Client
  participant MemPool as MemPoolAccept
  participant Validator as IsStandardSpecialTx
  participant Mempool
  Client->>MemPool: Submit asset lock transaction
  MemPool->>MemPool: PreChecks: check fRequireStandard
  alt fRequireStandard enabled
    MemPool->>Validator: Validate special transaction
    Validator->>Validator: Check if TRANSACTION_ASSET_LOCK
    alt Asset lock with ≤100 inputs, ≤20480 bytes, version<2
      Validator->>MemPool: return true
      MemPool->>Mempool: Accept to mempool
    else Exceeds limits or invalid version
      Validator->>MemPool: return false with reason
      MemPool->>Client: Reject TX_NOT_STANDARD
    end
  else fRequireStandard disabled
    MemPool->>Mempool: Accept to mempool
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• UdjinM6
• PastaPastaPasta

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: make huge asset lock tx non-standard' directly and concisely summarizes the main change: marking oversized asset lock transactions as non-standard to prevent relay on the network.
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.
Description check	✅ Passed	The pull request description clearly explains the issue (Platform ignores large asset-locks), the solution (marking them as non-standard), and references related work (PR #7294 and Platform PR #3491).

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[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 765-792: Extend test_non_standard to also exercise the new
serialized-size rule: construct an asset lock transaction whose serialized size
exceeds 20,480 bytes (use the existing helpers in the test — e.g., build a
funding/split tx with many outputs or add a large OP_RETURN/payload and call
create_assetlock to produce the oversized asset lock), assert its vin length if
useful, then call node_wallet.testmempoolaccept and nodes[1].testmempoolaccept
and verify the permissive node allows it but the standard node rejects it with
reject-reason 'assetlocktx-too-large' (follow the existing pattern used for
lock_v2 and lock_many_inputs in test_non_standard).

🪄 Autofix (Beta)

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

Run ID: 27836ae4-8a8a-4eb5-80ed-191953376c14

📥 Commits

Reviewing files that changed from the base of the PR and between 317917a and 5f0e2fcf971d7e3957c4779d9908a5a219021dd3.

📒 Files selected for processing (4)

• src/evo/specialtxman.cpp
• src/evo/specialtxman.h
• src/validation.cpp
• test/functional/feature_asset_locks.py

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Missing functional coverage for the assetlocktx-too-large standardness rule.

test_non_standard currently validates only two reject paths (Line 786): version 2 and >100 inputs. The PR introduces a third mempool standardness rule (serialized size > 20,480), but this test does not assert that rejection path, so that policy can regress unnoticed.

Suggested test extension

@@
-        for tx, reason in [(lock_v2, 'assetlocktx-version-2'), (lock_many_inputs, 'assetlocktx-too-many-inputs')]:
+        # TODO: also construct a tx with <=100 inputs but serialized size > 20480
+        # and assert reject-reason == 'assetlocktx-too-large'
+        for tx, reason in [
+            (lock_v2, 'assetlocktx-version-2'),
+            (lock_many_inputs, 'assetlocktx-too-many-inputs'),
+            # (lock_too_large, 'assetlocktx-too-large'),
+        ]:

🤖 Prompt for 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.

In `@test/functional/feature_asset_locks.py` around lines 765 - 792, Extend
test_non_standard to also exercise the new serialized-size rule: construct an
asset lock transaction whose serialized size exceeds 20,480 bytes (use the
existing helpers in the test — e.g., build a funding/split tx with many outputs
or add a large OP_RETURN/payload and call create_assetlock to produce the
oversized asset lock), assert its vin length if useful, then call
node_wallet.testmempoolaccept and nodes[1].testmempoolaccept and verify the
permissive node allows it but the standard node rejects it with reject-reason
'assetlocktx-too-large' (follow the existing pattern used for lock_v2 and
lock_many_inputs in test_non_standard).

[thepastaclaw]

Code Review

Codex's blocking finding is correct: the PR's stated goal is for v2 asset locks to be non-standard but consensus-valid, yet CAssetLockPayload::CURRENT_VERSION is still 1 and CheckAssetLockTx rejects any getVersion() > CURRENT_VERSION. CheckSpecialTx is invoked unconditionally inside MemPoolAccept::PreChecks (validation.cpp:959) and during block validation, so a v2 asset lock will be rejected via consensus regardless of the new policy gate — both nodes in the new functional test will fail to mine lock_v2. Both reviewers also converge on the missing functional coverage for the assetlocktx-too-big branch. Claude's nitpicks about the magic-number naming and 'silent pass-through' on bad payload deserialization were dropped: payload-deserialization failures are caught later in the same PreChecks invocation by CheckAssetLockTx ("bad-assetlocktx-payload"), so no relay leakage occurs.

🔴 1 blocking | 🟡 1 suggestion(s)

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/evo/specialtxman.cpp`:
- [BLOCKING] src/evo/specialtxman.cpp:1259-1263: v2 asset locks remain consensus-invalid, contradicting the PR goal and breaking the new test
  The PR's header doc and commit message state that v2 asset locks should be "non-standard but consensus-valid" so they can be activated later without another hard fork. The implementation only adds a relay/policy check in `IsStandardSpecialTx`, but does not relax consensus: `CAssetLockPayload::CURRENT_VERSION` is still 1 (src/evo/assetlocktx.h:31), and `CheckAssetLockTx` returns `bad-assetlocktx-version` for `getVersion() > CURRENT_VERSION` (src/evo/assetlocktx.cpp:56). `MemPoolAccept::PreChecks` calls `CheckSpecialTx` unconditionally at validation.cpp:959 (not gated by `fRequireStandard`), and `ProcessSpecialTxsInBlock` calls the same path during block validation. As a result:

  - `node_wallet.sendrawtransaction(lock_v2.serialize().hex())` in the new functional test (feature_asset_locks.py:794) will throw with `bad-assetlocktx-version` even on the permissive `-acceptnonstdtxn=1` node, so the test will fail.
  - Even if the tx is force-injected into a block, `CheckBlock`/`ConnectBlock` paths will reject the block as consensus-invalid.

  Either bump `CAssetLockPayload::CURRENT_VERSION` to 2 (and gate it on the appropriate deployment, e.g. v24), or remove the v2 standardness path from this PR until consensus actually accepts v2. As written, the PR cannot achieve its stated outcome and the functional test will not pass.

In `test/functional/feature_asset_locks.py`:
- [SUGGESTION] test/functional/feature_asset_locks.py:765-802: `assetlocktx-too-big` policy branch is not exercised
  `test_non_standard` covers `assetlocktx-version-2` and `assetlocktx-too-many-inputs`, but never builds an asset lock with ≤100 inputs and `GetTotalSize() > 20480`. In `IsStandardSpecialTx`, the input-count check runs before the size check (specialtxman.cpp:1248 → 1254), so the 101-input fixture short-circuits on `assetlocktx-too-many-inputs` and the 20 KB branch is dead from the test's perspective. Add a case that crafts an asset lock with ≤100 inputs whose serialized size exceeds 20480 bytes (e.g. by padding `creditOutputs` count or scriptSigs) and asserts the reject reason is `assetlocktx-too-big`. Otherwise a future reorder of the checks or a change to the `max_tx_size_for_platform` constant can regress silently.