Transparently call receiveCoins when an SC attempts to transferCoins towards another SC

[damip]

Context

In Massa, an SC can't currently transferCoins towards another SC, but has to call a payable function (all exported functions are payable by default). The de-facto standard is for smart contracts to export a function named receiveCoins.

Some smart contracts dynamically select the way they transfer depending on the nature of the target: https://github.com/massalabs/massa-name-service/blob/faf01258a094262ff162b656dc4b48d8209ff125/smart-contract/assembly/contracts/main.ts#L244-L250

Unfortunately that practice has not been implemented at large. In particular, it was not done for the WMAS contract which is now immutable. This means that a smart contract will never be able to unwrap a WMAS into MAS in order to pay fees for example.

This limitation strongly hampers autonomous applications and is currently blocking Dusa's autonomous DCA impementation.

Proposal

In this discussion I propose that we add a fix at the L1 level that does the following:

In the transferCoins ABI, if the target is an SC, instead of failing, transparently forward the execution towards a Call ABI that attempts to call the receiveCoins exported function of the target address. It would behave as if the SC called the target SC's receiveCoins function with coins = the amount of tokens being transferred, instead of purely attempting a coin transfer.

This would be clearly documented in the transferCoins ABI, and the `receiveCoins` function would be made a standard.

This would enable existing smart contracts that transfer coins to their caller to be operated not only by users but also by other smart contracts and would solve autonomy issues.

Comparison with Ethereum

In Ethereum, transfer is basically a sugar-coat for a CALL with a fixed gas limit and towards a pre-defined exported function name:

The transfer function also uses the CALL opcode, similar to send, but with a fixed gas stipend of 2300. It’s essentially an abstraction over CALL that automatically reverts the transaction if the transfer fails, providing an added layer of safety.

Use Case: We use transfer when we want to ensure that our Ether transfer either succeeds or reverts.

Security Consideration: transfer is safe from reentrancy attacks due to its low gas limit. However, the gas limit can cause issues when the contracts requires more gas to handle received Ether.

See https://dev.to/110nard0/understanding-ethereums-sendtransfercall-functions-2f1m

The idea is that a transfer always costs a predictable amount of gas, but a pretty high value.

I am personally not a big fan of the hardcoded gas limit idea because:

• it limits what the receiving SC can do when receiving coins
• it is supposed to defend against re-entrancy but this creates a strong coupling with seemingly unrelated constants: if in the future we decide to change gas calibration, we might unknowingly lift the re-entrancy protection

Overall, I propose we do the same as ETH but with dynamic gas, just document everything very clearly.

Limitations

Risk: unexpected gas usage

A smart contract using transfer might not expect that it possibly uses an arbtirary amount of gas.
To mitigate this, resorting to doing a first estimate with a read-only call is a good practice for users.
Otherwise, proper documentation of the ABI should suffice.

Risk: re-entrancy

This risk is linked to an SC attempting to unexpectedly call the receiveCoins function of a target SC that itself calls back the original SC, possibly triggering a re-entrancy attack. This risk is mitigated by properly documenting and warning that any call to transferCoins might trigger a call.

Making the Transfer operation work toward SCs

Could we also make the Transfer operation transparent so that EOAs can transfer coins to smart contracts from their wallet ?
Unfortunately, no because it would require hardccoding the max_gas parameter which is absent in a Transfer operation, and as explained above, it is something we want to avoid.
However, this could be abstracted away by wallet software that would automatically detect that a user is trying to transfer towards a smart contract and propose to call the receiveCoins function of the target SC instead.

[bilboquet]

Did we implement the opposite in #4027?
That's very unclear.

[sydhds]

re-entrancy is a big source of security issue (as listed here: https://github.com/kadenzipfel/smart-contract-vulnerabilities/blob/master/vulnerabilities/reentrancy.md) so are we sure we want to have this in Massa smart contract? @damip

[damip]

Did we implement the opposite in #4027? That's very unclear.

We prevented uncontrolled transfers towards SCs. Because SCs should be able to filter funds and not automatically accept all of them like EOAs do. So to transfer coins, smart contracts used the following trick https://github.com/massalabs/massa-name-service/blob/faf01258a094262ff162b656dc4b48d8209ff125/smart-contract/assembly/contracts/main.ts#L244-L250 to transfer funds to other contracts. The rule was that smart contracts wanting to receive coins generically would export the receiveCoins function. But this trick was not implemented everywhere. So the goal here is to still allow smart contracts to control incoming funds (the reason why we disabled transfers towards SCs stays valid) but to allow for a generic way of transparently transferring from SCs that did not implement the trick.

[damip]

re-entrancy is a big source of security issue (as listed here: https://github.com/kadenzipfel/smart-contract-vulnerabilities/blob/master/vulnerabilities/reentrancy.md) so are we sure we want to have this in Massa smart contract? @damip

Not more risky than a normal Call. We just need to document that fact. Ethereum tries to mitigate the issue a bit by limiting the gas of the transfer call but it's not perfect either.

[peterjah]

Can you explain this We prevented uncontrolled transfers towards SCs. Because SCs should be able to filter funds and not automatically accept all of them like EOAs do? I don't understand why there is this need.

One of the main limitation of the proposed solution is that the transfer will work ONLY if the contract has the "receiveCoin" function.
Which as you said is not the case for most of already deployed contracts. And it force to do additionnal code in smart contract for something that should be "by default".
As we are willing to modify the behaviour of the VM, why not directy add the ability to receive coins for a SC, without adding receiveCoin function??
It is also good to notice that if you do that, you remove the reentrency attack vector.

[Leo-Besancon]

why not directy add the ability to receive coins for a SC, without adding receiveCoin function??

I think the main issue is that if someone mistakenly sends coins to a contract that has no way to withdraw it, the coins are lost forever, that would be a bit problematic as the default case if it can be prevented.

[sydhds]

but a SC could check if the function "receiveCoin" exists before doing the call no?

[Leo-Besancon]

but a SC could check if the function "receiveCoin" exists before doing the call no?

Yes and that's the current way of doing it, but it can't be implemented already deployed immutable contracts such as WMAS

[peterjah]

I think the main issue is that if someone mistakenly sends coins to a contract that has no way to withdraw it, the coins are lost forever, that would be a bit problematic as the default case if it can be prevented.

Ok. for me its not shoking. and it would be the same as ethereum.

Furthermore, the probleme you describe is already here. users are sending coins through call SC, and most of the time there is nothing to get them back

[peterjah]

I mean, forcing every contract writter to add some code and logic to handle something that should be by default just because "someone could loose coins if its contract is wrongly coded". I doubt its the good approach.
For me its pretty obvious that smart contracts need to send and receive funds between them.

If we follow this logic, maybe we should forbid to send MRC20 tokens to a smart contract ? seems absurd

[bilboquet]

You mean you would (basically) revert the behavior before #4027?
If the sole drawback is:

"someone could loose coins if its contract is wrongly coded".
I second you (that was my intent while raising #4027)

[damip]

After all the valid points you guys made, I would like to reopen the idea of allowing transfers towards SCs instead of implementing this proposal.

[damip]

Got feedback from Dusa: OK for re-enabling transfers towards SCs.

Warning: if an SC address does not exist and a transfer is attempted towards it, we need to FAIL instead of creating the SC address like we do for EOA