Xahau / xahaud

Codebase for Xahaud - The consensus, RPC & blockchain app for the Xahau network.
https://xahau.network
ISC License
22 stars 11 forks source link

Remit does not call the token issuer's Hook as weakTSH. (Version: 2024.3.12-release+790 ) #297

Open tequdev opened 3 months ago

tequdev commented 3 months ago

Issue Description

If a token is specified in the Amounts field of a Remit transaction, the Hook of that token issuer will not be invoked.

Expected Result

The issuer of the token being sent must be weakTSH.

Actual Result

The issuer of the token being sent is not TSH.

Environment

xahaud 2024.3.12-release+790

Supporting Files

https://gist.github.com/tequdev/dd19dfaacca7bd3372b646f423004a03

RichardAH commented 3 months ago

good catch, an oversight indeed

will fix it on the next binary

dangell7 commented 3 months ago

If we add this to Remit then we need to decide if we should add this to payment as well. Related to https://github.com/Xahau/xahaud/issues/262

tequdev commented 3 months ago

I guess Payment and OfferCreate transactions set the weakTSH through addWeakTSHFromSandbox().

https://github.com/Xahau/xahaud/blob/d24c134612c36594f1c19de693ba03d0560a25f6/src/ripple/app/tx/impl/Transactor.cpp#L1448

tequdev commented 3 months ago

We need to support this in transactors (Escrow, Paychan, Check) that do not use PaymentSandbox.

tequdev commented 3 months ago

I guess Payment and OfferCreate transactions set the weakTSH through addWeakTSHFromSandbox().

https://github.com/Xahau/xahaud/blob/d24c134612c36594f1c19de693ba03d0560a25f6/src/ripple/app/tx/impl/Transactor.cpp#L1448

Ah, this is for non-issuer.

dangell7 commented 3 months ago

addWeakTSHFromSandbox executes on crossed account that are not the issuer. So I think this is a different issue? Also does Escrow, Paychan and Check use pathing?

tequdev commented 3 months ago

Yes, the result would be the same as the existing issue. Is this a issue that needs to be fixed? Or is this a feature that is expected to be added in the future?

It may affect the validator's vote.

dangell7 commented 3 months ago

Technically its a feature imo but if @RichardAH is open to it we could add it.

RichardAH commented 3 months ago

I think issuers should be weak tshes on all dealing in their currencies. It's over-looked in a bunch of places indeed.