ClaimReward transaction does not appear in Issuer's AccountTx (Version: 2023.12.29-release+689)

[tequdev]

Issue Description

ClaimReward transaction does not appear in Issuer's AccountTx

Steps to Reproduce

import { Client } from '@transia/xrpl'

const client = new Client('wss://xahau.org')


const main = async () => {
  await client.connect()
  
  const response = await client.request({
    command: 'account_tx',
    account: 'rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh'
  })
  
  console.log(response.result.transactions)
  await client.disconnect()
}

main()

ClaimReward transaction specified as Issuer.
https://xahscan.com/tx/4845C3D7B0CDC04C6A2DA53A723AE60DAC12E4F491AB93B881E919CD3FB849ED

Expected Result

When retrieving a transaction in account_tx command for an account specified as Issuer in a ClaimReward transaction, the retrieved result contains a ClaimReward transaction.

Actual Result

Does not contain.

Environment

Supporting Files

https://github.com/Xahau/xahaud/blob/dev/src/ripple/ledger/impl/ApplyStateTable.cpp#L646-L682

It might be a simple idea to add the following process.

        if (auto const optSleIssr{(*sle)[~sfIssuer]})
                threadTx(base, meta, *optSleIssr, mods, j);

This would affect NFTokenMint transaction, Import transaction, and URIToken related transactions.

[dangell7]

Related: #140

Thank you for finding this.

[dangell7]

This will need more thought. Adding your suggestion doesn't add the issuer to the thread because the Issuer is on the tx not the ledger entry metadata. The issuer is on the thread of the EmittedTxn though. So in this case when the user opts in or out, they are not on the tx thread but when a GenesisMint occurs they are or should be. Further testing is required to confirm this.

Your suggestion however does add the issuer to the thread for URITokens which might be something we want.

Finally, the issuer on an import txn is already on the thread through the EmittedTxn entry thats created. So it should already show there.

[tequdev]

Technically, ClaimReward is similar to Invoke and is intended to invoke Hook.
It seems to me that sfIssuer is which Hook to invoke and should work the same way as sfDestination in an Invoke transaction.

So in my opinion, even if there is no Hook in sfIssuer, the Issuer should be added to the thread, and if so, there must be a disallowIncomingClaimReward.

Finally, the issuer on an import txn is already on the thread through the EmittedTxn entry thats created. So it should already show there.

I didn't check up to see what it was current, and only listed the transactions that were affected by the field name.

[dangell7]

Technically, ClaimReward is similar to Invoke and is intended to invoke Hook. It seems to me that sfIssuer is which Hook to invoke and should work the same way as sfDestination in an Invoke transaction.

So in my opinion, even if there is no Hook in sfIssuer, the Issuer should be added to the thread, and if so, there must be a disallowIncomingClaimReward.

Finally, the issuer on an import txn is already on the thread through the EmittedTxn entry thats created. So it should already show there.

I didn't check up to see what it was current, and only listed the transactions that were affected by the field name.

Yes currently it is exactly like Invoke, and neither of them are shown in account_tx and we should fix that. I'm just saying the solution you provided doesn't fix that and I will need to do more work to solve it.

Can you explain more about the disallowIncomingClaimReward? The user would set this if they dont want to receive claim rewards? Cant they just opt out or not submit the claim reward txn? Or the issuer sets this?

[tequdev]

Just to prevent unwanted transactions from appearing in the transaction history of the account that is set in Issuer.

[dangell7]

Just to prevent unwanted transactions from appearing in the transaction history of the account that is set in Issuer.

Isn't the genesis account the issuer? So a hook would need to be voted in by the governance board to set this hook I believe. That seems like a lot of extra work when there are other priorities. If you want to PR that then please by all means.

However please keep in mind that would mean that while every other disallowIncoming flag is used to block a transaction, this flag only removes it from the account_tx history. I think that is confusing so the flag would need to be named something else. Furthermore, looking up the account sle will also add complexity to every rpc call so you should weigh the pros and cons.