[XLS-73] Incorrect error in AMMClawback: temINVALID_FLAG returned instead of temMALFORMED

[HugoDowsers]

Issue Description

While validating an AMMClawback transaction, if a transaction contains both an incorrect asset issuer (Asset.issuer) and an invalid usage of the tfClawTwoAssets flag, the system returns temINVALID_FLAG instead of temMALFORMED.

XLS-73 specifies that failing the tfClawTwoAssets validation (when the second asset is not issued by the sender) should return tecNO_PERMISSION rather than temINVALID_FLAG.

rippled/src/xrpld/app/tx/detail/AMMClawback.cpp

Line 48 in 0a626d9

return temINVALID_FLAG;

https://github.com/XRPLF/XRPL-Standards/tree/master/XLS-0073-amm-clawback

Steps to Reproduce

• Submit an AMMClawback transaction where the Account field (the sender) does not match the issuer defined in the Asset field (Violation of Rule S2).
• Simultaneously enable the tfClawTwoAssets flag while the second asset (Asset2) is issued by a third party or is XRP (Violation of Rule S3).

Expected Result

The transaction should return temMALFORMED. The failure to validate the primary asset's issuer (Asset.issuer == Account) is an error that must be detected before any secondary flag analysis is performed.

Actual Result

The transaction returns temINVALID_FLAG. The current code in AMMClawback::preflight evaluates the condition (flags & tfClawTwoAssets && asset.account != asset2.account) before the mandatory check (asset.account != issuer).

Environment

OS: Ubuntu 22.04 LTS
Compiler: GCC 11 / Clang 14
Boost: 1.77+
Version: xrpld develop branch (post XLS-73 integration)

Supporting Files

None.

[yinyiqian1]

@HugoDowsers Thank you for raising this.

We’ll need to update the spec — this should return temINVALID_FLAG.

We can return the tem code early during preflight. At that stage, we already have access to both Asset and Asset2, so we can determine whether they share the same issuer. If the issuers differ, then tfClawTwoAssets should be treated as an invalid flag.

We prefer performing malformed transaction checks in preflight. This prevents the malformed transaction from being broadcast to the network and avoids unnecessary transaction fees.

[HugoDowsers]

Thanks for the feedback, I'm trying to do some formal verification and this kind of spec/code disagreement comes up from time to time. I'll try to post other cases like this when they occur

[yinyiqian1]

Thanks for the feedback, I'm trying to do some formal verification and this kind of spec/code disagreement comes up from time to time. I'll try to post other cases like this when they occur

I will go through the AMMClawback specification to verify consistency with the implementation and will provide an updated version in the coming days.