OpenZeppelin / cairo-contracts

OpenZeppelin Contracts written in Cairo for Starknet, a decentralized ZK Rollup
https://docs.openzeppelin.com/contracts-cairo
MIT License
797 stars 322 forks source link

[TOB] TOB-ACCOUNT-2: Error messaging is obscure due to use of unwrap #890

Closed technovision99 closed 4 months ago

technovision99 commented 5 months ago

Error messaging is obscure due to use of unwrap

Severity: Informational Difficulty: High Type: Error Reporting Target: src/account/utils.cairo

Description

After the transaction has been validated, the Starknet OS will call the __execute__ function. This function allows the account contract to make multiple calls to other contracts, reverting if one of the calls is unsuccessful. To do so, the execute_calls function will call execute_single_call in a loop. The latter function then calls a contract with the call_contract_syscall. However, instead of unwrapping the result with unwrap_syscall, which provides the reason for the revert, unwrap is used, which does not provide a meaningful error message.

https://github.com/OpenZeppelin/cairo-contracts/blob/ef4128c61c6104852a1172e0e43e22fce1d33075/src/account/utils.cairo#L29-L32

Recommendations

Short term, use unwrap_syscall instead of unwrap.

Long term, ensure that any state reversions provide adequate error messaging in order to help end users understand the underlying problem.

andrew-fleming commented 4 months ago

Resolved with #897