Skip to content

Commit

Permalink
chore: revert the check to allow arbitrary calldata length use cases (#…
Browse files Browse the repository at this point in the history
…57)

## Summary
The calling contract should be responsible for checking the calldata
length when they call `executeFromPluginToExternal`.

## Detail
### Changeset
* reverted the change and added a new test.

### Checklist
- [x] Did you add new tests and confirm all tests pass? (`yarn test`)
- [ ] Did you update relevant docs? (docs are found in the `docs`
folder)
- [x] Do your commits follow the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) standard?
- [x] Does your PR title also follow the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) standard?
- [ ] If you have a breaking change, is it [correctly reflected in your
commit
message](https://www.conventionalcommits.org/en/v1.0.0/#examples)? (e.g.
`feat!: breaking change`)
- [x] Did you run lint (`yarn lint`) and fix any issues?
- [x] Did you run formatter (`yarn format:check`) and fix any issues
(`yarn format:write`)?

## Testing
* added a test case to verify native token transfer is allowed

## Documentation
n/a
  • Loading branch information
huaweigu authored Dec 20, 2024
1 parent 9f8c6de commit af55d55
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 9 deletions.
3 changes: 0 additions & 3 deletions src/msca/6900/v0.7/managers/PluginExecutor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,6 @@ library PluginExecutor {
internal
returns (bytes memory)
{
if (data.length < 4) {
revert NotFoundSelector();
}
if (target == address(this) || ERC165Checker.supportsInterface(target, type(IPlugin).interfaceId)) {
revert ExecuteFromPluginToExternalNotPermitted();
}
Expand Down
10 changes: 6 additions & 4 deletions test/msca/6900/v0.7/PluginExecutor.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -1186,6 +1186,8 @@ contract PluginExecutorTest is TestUtils {
function testShortCalldataIntoExecuteFromPluginToExternal() public {
// deployment was done in setUp
assertTrue(address(msca).code.length != 0);
// start with balance
vm.deal(address(msca), 10 ether);
bytes32 manifestHash = keccak256(abi.encode(testTokenPlugin.pluginManifest()));
// airdrop 1000 tokens
bytes memory pluginInstallData = abi.encode(1000);
Expand All @@ -1195,11 +1197,11 @@ contract PluginExecutorTest is TestUtils {
dependencies[0] =
FunctionReference(address(singleOwnerPlugin), uint8(ISingleOwnerPlugin.FunctionId.USER_OP_VALIDATION_OWNER));
msca.installPlugin(address(testTokenPlugin), manifestHash, pluginInstallData, dependencies);

// fail early even before InvalidValidationFunctionId is reverted
vm.expectRevert(NotFoundSelector.selector);
bytes memory data = abi.encodeCall(testTokenPlugin.callExecuteFromPluginExternal, (bytes("aaa")));
address externalTarget = testTokenPlugin.LONG_LIQUIDITY_POOL_ADDR();
uint256 initialBal = externalTarget.balance;
bytes memory data = abi.encodeCall(testTokenPlugin.callExecuteFromPluginExternal, (1, bytes("")));
address(msca).callWithReturnDataOrRevert(0, data);
assertEq(externalTarget.balance, initialBal + 1);
vm.stopPrank();
}
}
5 changes: 3 additions & 2 deletions test/msca/6900/v0.7/TestTokenPlugin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ contract TestTokenPlugin is BasePlugin {
return true;
}

function callExecuteFromPluginExternal(bytes calldata data) external returns (bool) {
IPluginExecutor(msg.sender).executeFromPluginExternal(LONG_LIQUIDITY_POOL_ADDR, 0, data);
function callExecuteFromPluginExternal(uint256 value, bytes calldata data) external returns (bool) {
IPluginExecutor(msg.sender).executeFromPluginExternal(LONG_LIQUIDITY_POOL_ADDR, value, data);
return true;
}

Expand Down Expand Up @@ -517,6 +517,7 @@ contract TestTokenPlugin is BasePlugin {
})
});

manifest.canSpendNativeToken = true;
manifest.dependencyInterfaceIds = new bytes4[](1);
manifest.dependencyInterfaceIds[0] = type(ISingleOwnerPlugin).interfaceId;
return manifest;
Expand Down

0 comments on commit af55d55

Please sign in to comment.