gno icon indicating copy to clipboard operation
gno copied to clipboard

`grc20.Token` doesn't implement `IGRC20` interface

Open lumtis opened this issue 2 years ago • 3 comments

Maybe this part is under active development. grc20.Token in https://github.com/gnolang/gno/blob/master/examples/gno.land/p/grc/grc20/grc20.gno doesn't implement the IGRC20 as it seems the interface has been modified

I wanted to make a quick fix but I'm unsure of some changes like for Transfer(to std.Address, amount uint64), does the plan is to call GetCallerAt in Token implementation?

Some methods also return boolean for successful completion, wouldn't be better to use an error object?

lumtis avatar Aug 14 '22 19:08 lumtis

hey @lubtd,

yes, this part is under active development, but that’s the reason why your interest can be useful.

There is a tradeoff to choose between being as close as possible to the ERC20 standard and making something allowing the same features while being more pure-gno. There is also the idea of making multiple implementations and interfaces to manage both worlds together.

Would love to get your feeling to complete the list of discussions I already had on the topic. Thank you very much!

moul avatar Aug 15 '22 21:08 moul

I would be in favor of implementing the current interface from IGRC20, not in the idea of being as close as possible to the ERC20 standard but because it makes a simpler implementation.

Having owner parameter for Transfer seems like unnecessary unless there is an idea of a privilege granting system from account to account like authz module in Cosmos SDK https://docs.cosmos.network/master/modules/authz/,

Are there concerns about using GetCallerAt in the object method? is it related to what you're referring to by pure-gno?

I would also keep the return values for the methods to have a more intuitive API but change bool to error

lumtis avatar Sep 04 '22 15:09 lumtis

GetCallerAt(x) could easily introduce bugs to people who program in gnolang. It is also hard for people to audit the contract.

For example, we use GetCallerAt(2) in many example contracts to check the original caller logic. However, it equals to origin caller if and only if the caller is the immediate caller of the current contract we are implementing. In other words, the original caller is on the 2nd frame on the call stack.

The problem: What if the original caller does not call directly to the contract we are implementing? The original caller could be on the 3rd or 4th frame on the call stack. Therefore, our verification logic could fail.

Here are some discussion points about this topic.

  1. Think about why we need to differentiate callers on the call stack when we implement a contract in the first place?

  2. Let's say the call flow is ContractA > B > C > D. Do we need to identify all callers on the call stack? For example, what will happen if the call stack is A > ... > Z?

So far, there are use cases like bank and brokerage contracts in that we need to identify original caller A and immediate caller C before D.

  1. If we indeed to get a hold of all callers on the call stack, is there a better way to reference it than GetCallerAt(x)?

  2. One proposal: We only reference the caller through std.GetOrignalCaller() and std.GetImmediateCaller() to cover most of the verification scenarios for contracts that provide and consume on-chain services. Also, we can introduce the get caller by realm package name on the call stack to guard the call path further explicitly.

piux2 avatar Sep 13 '22 20:09 piux2