Atlas: bubble app error data while preserving 4-byte Atlas selector#500
Atlas: bubble app error data while preserving 4-byte Atlas selector#5000x1NotMe wants to merge 4 commits intoatlas-v1.6.3from
Conversation
…ving 4-byte selector - Escrow: append app revert to PreOpsFail/SimFail, UserOpFail/SimFail, AllocateValueFail/SimFail - Atlas._handleErrors: forward full revert payload for these errors (sim + non-sim) - First 4 bytes remain Atlas error selector to preserve decoding semantics
…or selectors for delegatecall/call failures and helper balanceOf errors
…bbling in preOps failure
…ver balanceOf revert
| assembly { | ||
| mstore(0, _errorSwitch) | ||
| revert(0, 4) | ||
| revert(add(revertData, 32), mload(revertData)) |
There was a problem hiding this comment.
FYI these changes push Atlas over the contract size limit by about 185 bytes. If we go with these changes we will need to lower optimizer_runs to get it back under.
But also try to cut down contract size in the modified areas if possible. There might be a Solady lib function that could do this, not sure
There was a problem hiding this comment.
Yeah, there's this helper. We should probably use it once we've build the bytes revertData in each case we want to bubble up
https://github.com/Vectorized/solady/blob/main/src/utils/LibCall.sol#L201
There was a problem hiding this comment.
I don't think this would actually reduce contract size but I can add the fix
There was a problem hiding this comment.
183 bytes without Solady and 209 bytes over the limit using LibCall.bubbleUpRevert
There was a problem hiding this comment.
Oof okay. Was hoping it would reduce code duplication.
- Replace confusing variable names (_err2 -> _err, _ret -> _returnData) in ExecutionEnvironment.sol - Update test naming convention to follow project standards (test_ErrorHandling_*) - Replace try-catch pattern with low-level call pattern in tests for cleaner code - Remove console.log statements from tests - Add comprehensive test coverage for all error bubbling scenarios: - PreOps failures - UserOp delegatecall and regular call failures - AllocateValue failures - PreSolver and PostSolver failures - Security test for malicious solver error spoofing - Fix test configuration to include proper gas limits for all UserOperations These changes address all feedback from PR #500 while maintaining the core functionality of preserving Atlas error selectors with appended DApp error data. Co-Authored-By: BenSparksCode <noreply@github.com>
0x1NotMe
force-pushed
the
feat/atlas-bubble-app-errors
branch
from
0a1429d to
b06c22b
Compare
BenSparksCode
left a comment
BenSparksCode
left a comment
There was a problem hiding this comment.
Even after setting optimizer runs to the lowest (1), Atlas is still 55 bytes over the limit, so I don't think we can merge this change in - no way we can deploy it on most EVM chains without refactoring, or the contract size limit EIP upgrade
2 participants
Summary
Rationale
Notes