allow_reentrancy modifier for messages #1285
Description
To prevent developers from making reentrancy vulnerabilities in the contract ink! doesn't allow reentrancy during cross-contract calls by default. It is controlled via CallFlags.allow_reentry(false by default) during the creation of a cross-contract call(CallBuilder).
/// The flags used to change the behavior of a contract call.
#[must_use]
#[derive(Copy, Clone, Debug, Default)]
pub struct CallFlags {
forward_input: bool,
clone_input: bool,
tail_call: bool,
allow_reentry: bool,
}If the contract already exists in the stack of calls, the contract-pallet immediately finishes executing the transaction and throws the ReentranceDenied error. The caller contract can't handle it, and it is hard to debug(discussion). Also, with that workflow, we can't have some functions that allow reentrancy and some that deny it.
To improve the usability and add control on which methods accept reentrancy and which do not, we want to introduce a new modifier for the message allow_reentrancy and change the default behavior of the CallFlags.
A new definition looks like:
/// The flags used to change the behavior of a contract call.
#[must_use]
#[derive(Copy, Clone, Debug, Default)]
pub struct CallFlags {
forward_input: bool,
clone_input: bool,
tail_call: bool,
deny_reentry: bool, // reverse behavior
}So it is a breaking change because the allow_reentry function will be renamed to deny_reentry(We can keep empty allow_reentry, but better inform all users about the new API).
The allow_reentrancy behaves as a payable modifier but checks the output of seal_reentrant_count. All methods don't allow reentrancy by default. But the developer can mark some methods with the allow_reentrancy modifier to allow it.
/// Transfers `amount` from caller to `to`.
#[ink(message, allow_reentrancy)]
fn transfer(&mut self, to: AccountId, amount: Balance);The implementation in codegen is the same as for payable so we need only repeat the logic.