ink
ink copied to clipboard
`allow_reentrancy` modifier for messages
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.
We plan to create a pull request into ink! after merging https://github.com/paritytech/substrate/pull/11539.
Are we okay with a new API and behavior? The level of security is the same. The caller contract controls the reentrancy during the execution of the cross-contract call(if we enter again into the same contract) than during instantiating the cross-contract call.