[POC] Demo: Standard Error Wrapping #244
Conversation
| constructor(IPoolManager _poolManager) SafeCallback(_poolManager) {} | ||
|
|
||
| /// @notice internal function that triggers the execution of a set of actions on v4 | ||
| /// @dev inheriting contracts should call this function to trigger execution | ||
| function _executeActions(bytes calldata unlockData) internal { | ||
| poolManager.unlock(unlockData); | ||
| try poolManager.unlock(unlockData) {} | ||
| catch (bytes memory error) { |
There was a problem hiding this comment.
I would suggest renaming error because it's a reserved keyword
There was a problem hiding this comment.
we usually use reason... and it will also match the input in the bubble up error definitions
| constructor(IPoolManager _poolManager) SafeCallback(_poolManager) {} | ||
|
|
||
| /// @notice internal function that triggers the execution of a set of actions on v4 | ||
| /// @dev inheriting contracts should call this function to trigger execution | ||
| function _executeActions(bytes calldata unlockData) internal { | ||
| poolManager.unlock(unlockData); | ||
| try poolManager.unlock(unlockData) {} | ||
| catch (bytes memory error) { |
There was a problem hiding this comment.
we usually use reason... and it will also match the input in the bubble up error definitions
| try poolManager.unlock(unlockData) {} | ||
| catch (bytes memory error) { | ||
| if (bytes4(error) == IPoolManager.CurrencyNotSettled.selector) { | ||
| revert Wrap__CurrencyNotSettled(address(poolManager), error); |
There was a problem hiding this comment.
why are we calling out this one in particular as different? even though its being bubbled too?
There was a problem hiding this comment.
just picked this one as an easy revert to catch individually, gonna confer with daniel on whether we need to individually handle reverts as best as we can vs using the generic catchall handler
|
upon further discussions, we agreed that a contract should only taking a look at posm, we are not making any calls to arbitrary addresses until subscriber is merged in. closing for now, but will reference in the future |
Related Issue
Exploring the ergonomics of wrapped error handling
Description of changes