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

NEP5 transferFrom should use originator #17

Open
SilverDragon135 opened this issue Mar 1, 2018 · 2 comments
Open

NEP5 transferFrom should use originator #17

SilverDragon135 opened this issue Mar 1, 2018 · 2 comments

Comments

@SilverDragon135
Copy link

Hi, maybe aesthetic issue. I wonder if it is good idea to let anyone to call transferFrom.

I think it is good idea to CheckWitness(t_to) to avoid unexpected assets flow. Since I can subscribe to OnApprove(), I can see all the approvals and that way I can do "rogue" transferFrom. It is not issue in the context of assets owner, but lets see simple example:

I have two NEO accounts. Family and personal. I want to have funds from family acc accessible to my personal account, if I need them, but still separated.

In case someone starts automatically transferring trough transferFrom (to make it more evil transfers in decimals), I would lost the separability and overview about flow of my funds. In the case, that multiple family members have approvals to that account, they would be cut from the family funds.

Definitely it is not critical, but I can imagine, that it can cause issues in specific situations.

@jeisses
Copy link

jeisses commented Mar 1, 2018

Hi SilverDragon,

You are right, but I think there is a flaw in the implementation of do_transfer_from. The transferFrom function should allow transfers from "owner" by an "originator" to any "to" address, according to the NEP5 proposal. Which means there should be a CheckWitness on the originator to prevent rogue transfers. I'll create a pull request that shows this concept

@SilverDragon135 SilverDragon135 changed the title NEP5 transferFrom should CheckWitness NEP5 transferFrom should use originator Mar 1, 2018
@brianlenz
Copy link

See my comment in #19 for why I believe the behavior of transferFrom should remain unchanged.

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

No branches or pull requests

3 participants