search

nash-io / neo-ico-template

An ICO Template for NEO projects
GNU General Public License v3.0
117 stars 56 forks source link

NEP5 transferFrom should use originator #17

Open SilverDragon135 opened 6 years ago

SilverDragon135 commented 6 years ago

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 commented 6 years ago

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

brianlenz commented 6 years ago

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