Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
tequdev opened this issue Mar 16, 2024 · 10 comments
Labels
feature Feature Request

Comments

@tequdev
Copy link
Collaborator

tequdev commented Mar 16, 2024

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
Copy link
Contributor

good catch, an oversight indeed

will fix it on the next binary

@dangell7
Copy link
Collaborator

dangell7 commented Mar 16, 2024

If we add this to Remit then we need to decide if we should add this to payment as well. Related to #262

@dangell7 dangell7 added the feature Feature Request label Mar 16, 2024
@tequdev
Copy link
Collaborator Author

tequdev commented Mar 16, 2024

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

Transactor::addWeakTSHFromSandbox(detail::ApplyViewBase const& pv)

@tequdev
Copy link
Collaborator Author

tequdev commented Mar 16, 2024

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

@tequdev
Copy link
Collaborator Author

tequdev commented Mar 16, 2024

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

Transactor::addWeakTSHFromSandbox(detail::ApplyViewBase const& pv)

Ah, this is for non-issuer.

@dangell7
Copy link
Collaborator

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
Copy link
Collaborator Author

tequdev commented Mar 16, 2024

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
Copy link
Collaborator

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

@RichardAH
Copy link
Contributor

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

@tequdev
Copy link
Collaborator Author

tequdev commented Oct 29, 2024

When is it appropriate to handle the issuer as a weakTSH?

A. when the IOU actually moves (eg. CheckCash)
B. when an IOU is specified in the transaction field (eg. CheckCreate)
C. when an Account field containing IOUs exists in the leisure object and they have changed (eg. CheckCancel)

The strictest will be A only and the loosest will be A+B+C.

The choices would be A or A+B or A+B+C

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature Request
Projects
None yet
Development

No branches or pull requests

3 participants