v3-periphery icon indicating copy to clipboard operation
v3-periphery copied to clipboard

Fix decoding error in Multicall

Open maAPPsDEV opened this issue 3 years ago • 4 comments

Summary Of Changes

This PR aims to fix the error in decoding revert reasons that was incorrectly implemented.

Technical Description

Early, a correct solution has been suggested here. But, the solution enforces it encodes the revert reason again when it finally revert at the end. Rather, it doesn't need to repeat decoding and encoding. One of the best efficient solutions might be in OpenZeppelin's Address.sol

Deployment Notes

N/A

Closes #254

maAPPsDEV avatar Apr 07 '22 12:04 maAPPsDEV

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale[bot] avatar Jul 14 '22 09:07 stale[bot]

Hi @fo1981 Thank you for reviewing it. Can I ask if it will be merged soon?

maAPPsDEV avatar Jul 14 '22 13:07 maAPPsDEV

We completely agree, within our clients we recommend updating the multicall revert to:

assembly {
    revert(add(result,32), mload(result))
}

Just blindly passing the revert bytes seems the most desirable solution in our opinion. The uniswap multicall only works with revert("string") errors and not with revert error(); errors, which in our opinion seems highly undesirable. It also does weird assembly magic as it shifts the error signature into the result.length, which is obviously undesirable.

JorgeAtPaladin avatar Jul 22 '22 14:07 JorgeAtPaladin

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale[bot] avatar Sep 20 '22 15:09 stale[bot]