Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[POC] Demo: Standard Error Wrapping #244

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .forge-snapshots/BaseActionsRouter_mock10commands.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
61756
61817
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_burn_empty.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
46764
46778
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_burn_empty_native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
46582
46596
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_burn_nonEmpty.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129410
129424
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_burn_nonEmpty_native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
122332
122345
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_collect.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
149444
149461
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_collect_native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
140596
140613
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_collect_sameRange.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
149444
149461
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_decreaseLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
115030
115044
Original file line number Diff line number Diff line change
@@ -1 +1 @@
107952
107965
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_decrease_burnEmpty.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
133169
133183
Original file line number Diff line number Diff line change
@@ -1 +1 @@
125908
125922
Original file line number Diff line number Diff line change
@@ -1 +1 @@
127703
127720
Original file line number Diff line number Diff line change
@@ -1 +1 @@
150923
150940
Original file line number Diff line number Diff line change
@@ -1 +1 @@
132723
132740
Original file line number Diff line number Diff line change
@@ -1 +1 @@
133650
133667
Original file line number Diff line number Diff line change
@@ -1 +1 @@
169806
169823
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
370958
370975
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
335658
335675
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_nativeWithSweep.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
344167
344184
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_onSameTickLower.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
313640
313657
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_onSameTickUpper.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
314282
314299
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_sameRange.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
239864
239881
Original file line number Diff line number Diff line change
@@ -1 +1 @@
368860
368877
Original file line number Diff line number Diff line change
@@ -1 +1 @@
319658
319675
Original file line number Diff line number Diff line change
@@ -1 +1 @@
415321
415338
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_Bytecode.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
6848
7305
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn1Hop_nativeIn.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
120831
120848
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn1Hop_nativeOut.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
119998
120015
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn1Hop_oneForZero.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
128870
128887
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn1Hop_zeroForOne.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
135700
135717
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn2Hops.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
187052
187069
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn2Hops_nativeIn.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
179015
179032
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn3Hops.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
238431
238448
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn3Hops_nativeIn.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
230418
230435
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactInputSingle.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
134585
134602
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactInputSingle_nativeIn.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
119716
119733
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactInputSingle_nativeOut.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
118861
118878
Original file line number Diff line number Diff line change
@@ -1 +1 @@
126634
126651
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut1Hop_nativeOut.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
120839
120856
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut1Hop_oneForZero.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129711
129728
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut1Hop_zeroForOne.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
134512
134529
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut2Hops.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
186473
186490
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut2Hops_nativeIn.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
183396
183413
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut3Hops.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
238476
238493
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut3Hops_nativeIn.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
235423
235440
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut3Hops_nativeOut.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
229628
229645
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOutputSingle.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
133389
133406
Original file line number Diff line number Diff line change
@@ -1 +1 @@
125511
125528
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOutputSingle_nativeOut.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
119774
119791
12 changes: 11 additions & 1 deletion src/base/BaseActionsRouter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,22 @@ abstract contract BaseActionsRouter is SafeCallback {
/// @notice emitted when an inheriting contract does not support an action
error UnsupportedAction(uint256 action);

error Wrap__SubcontextReverted(address subcontext, bytes reason);
error Wrap__CurrencyNotSettled(address subcontext, bytes reason);

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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest renaming error because it's a reserved keyword

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we usually use reason... and it will also match the input in the bubble up error definitions

if (bytes4(error) == IPoolManager.CurrencyNotSettled.selector) {
revert Wrap__CurrencyNotSettled(address(poolManager), error);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we calling out this one in particular as different? even though its being bubbled too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

} else {
revert Wrap__SubcontextReverted(address(poolManager), error);
}
}
}

/// @notice function that is called by the PoolManager through the SafeCallback.unlockCallback
Expand Down
42 changes: 42 additions & 0 deletions test/position-managers/PositionManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,48 @@ contract PositionManagerTest is Test, PosmTestSetup, LiquidityFuzzers {
}
}

function test_revert_wrapCurrencyNotSettled() public {
PositionConfig memory config = PositionConfig({poolKey: key, tickLower: -600, tickUpper: 600});
uint256 liquidity = 100e18;

// mint without settling
Plan memory planner = Planner.init();
planner.add(Actions.MINT_POSITION, abi.encode(config, liquidity, address(this), ZERO_BYTES));
bytes memory calls = planner.encode();

vm.expectRevert(
abi.encodeWithSelector(
BaseActionsRouter.Wrap__CurrencyNotSettled.selector,
address(manager),
abi.encodeWithSelector(IPoolManager.CurrencyNotSettled.selector)
)
);
lpm.modifyLiquidities(calls, _deadline);
}

/// @dev Test out SubcontextReverted on PoolNotInitialized
function test_revert_wrapSubcontextReverted() public {
// this pool was not initialized
key = PoolKey({currency0: currency0, currency1: currency1, fee: 0, tickSpacing: 10, hooks: IHooks(address(0))});

PositionConfig memory config = PositionConfig({poolKey: key, tickLower: -600, tickUpper: 600});
uint256 liquidity = 100e18;

// minting on a pool that doesnt exist
Plan memory planner = Planner.init();
planner.add(Actions.MINT_POSITION, abi.encode(config, liquidity, address(this), ZERO_BYTES));
bytes memory calls = planner.encode();

vm.expectRevert(
abi.encodeWithSelector(
BaseActionsRouter.Wrap__SubcontextReverted.selector,
address(manager),
abi.encodeWithSelector(IPoolManager.PoolNotInitialized.selector)
)
);
lpm.modifyLiquidities(calls, _deadline);
}

function test_initialize() public {
// initialize a new pool and add liquidity
key = PoolKey({currency0: currency0, currency1: currency1, fee: 0, tickSpacing: 10, hooks: IHooks(address(0))});
Expand Down
Loading