ink icon indicating copy to clipboard operation
ink copied to clipboard

`contract_ref!` produces traits with too strong requirements

Open deuszx opened this issue 1 year ago • 2 comments

Since contract_ref! macro produces the "access trait" with the exact same method signatures as the ones on the original contract, sometimes it puts too strong of requirements on self on the caller's side.

Consider the following case:

let mut psp22_ref: ink::contract_ref!(PSP22) = (*reward_token).into();
let balance: Balance = psp22_ref.balance_of(Self::env().account_id());
if balance > 0 {
  psp22_ref.transfer(running.owner, balance)?;
}

compiler requires that psp22_ref is mut because PSP22::transfer(&mut self, ...). Even though it should not be necessary on the call side since psp22_ref is just an address and not an actual contract state.

deuszx avatar Aug 07 '23 08:08 deuszx

I agree that we don't need to require a mut ownership level because, under the hood, we call a host function that behaves similarly for "read" and "write" methods.

But on the other side, it adds clarity to actions. When you call the mut method, you know it would change things on the callee side. It is not a problem to use ink::contract_ref! in the methods because you can always add mut to the variables showing that you want to modify the internal state of the callee object.

The problem comes when you want to store ink::contract_ref! as a field of your contract. Calling transfer in your case will force your message method also to be &mut self. You have two options here to avoid &mut self: work with AccountId and create ink::contract_ref! on demand; copy this variable with clone and mutate the copy.

It is possible to alter the trait/method by the ink macro during definition and replace all &mut self with &self. But it will affect every part of the code generation and add a "magic" behavior to each method.

I prefer to avoid that and keep things simple in a Rust way. A clone of the AccountId is a cheap operation, but you follow all rules defined by the Rust and don't have any unusual behavior (a.k.a magic).

xgreenx avatar Aug 08 '23 12:08 xgreenx

I don't think the knowledge of whether the method mutates the called contract or not is useful in more ways than simply theoretical. I doubt SC authors care about that. Even if the call is potentially dangerous (reentrancy attack) it can still be read-only (non-mutable). SC calling is too different from "normal Rust" for it to matter IMHO.

If you think it's too much work or would add too much "magic" (and/or gotchas) then these arguments convince me more than the one about having assumptions in the type-system.

deuszx avatar Aug 18 '23 11:08 deuszx