Repository navigation
fix(paymaster): compile the calldata of the calls like execute(), in getOutsideCall and in Contract - #1736
Merged
Conversation
…getOutsideCall and in Contract Since 11.4.1, the paymaster safety check rebuilds the expected outside execution from the user calls. getOutsideCall kept an array calldata as it was, so a parameter given as an object, like cairo.uint256(100), reached the hash and failed with "Invalid BigNumberish". contract.invoke, contract.estimate and withOptions with paymasterDetails hit it with any u256 object argument. account.getOutsideTransaction had the same problem. getOutsideCall now follows the rule of execute(): a calldata marked as compiled is kept, and any other calldata is a list or an object of parameters, which is flattened. For a flat calldata, the values do not change: they are only written in decimal in the OutsideCall. The paymaster branch of contract.invoke and contract.estimate sent the raw arguments. It now sends the calldata compiled with the abi, like execute(). Before, a u256 given as a number, as in transfer(recipient, 100), was sent as one felt instead of two. New unit tests cover both cases, and the Sepolia test contractPaymaster.test.ts passes again.
Contributor
|
🎉 This PR is included in version 11.4.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation and Resolution
This PR fixes two bugs of the paymaster path. The first one is a regression of 11.4.1.
1. A parameter given as an object broke the paymaster safety check. Since 11.4.1 (#1734), the safety check rebuilds the expected outside execution from the user calls, with
getTypedDataandgetOutsideCall.getOutsideCallkept an array calldata as it was. So a parameter given as an object, likecairo.uint256(100), reached the typed-data hash. The transaction then failed before signing, withInvalid BigNumberish: [object Object]. This hitcontract.invoke(…, { paymasterDetails }),contract.withOptions({ paymasterDetails }), andaccount.executePaymasterTransactionwith such a call.account.getOutsideTransactiongoes through the same code, and had the same problem.Resolution:
getOutsideCallnow follows the rule ofexecute()(seefromCallsToExecuteCalldata_cairo1). A calldata marked as compiled is kept as it is. Any other calldata is a list or an object of parameters, and is compiled withCallData.compile.2.
Contractsent the raw arguments to the paymaster. Incontract.invokeandcontract.estimate, the paymaster branch built the call with the raw arguments, not with the calldata compiled with the ABI. For example, withtransfer(recipient, 100), the u256 amount was sent as one felt instead of two.Resolution: both branches now send the call compiled with the ABI, the same call that
execute()uses.RPC version (if applicable)
Not applicable.
Usage related changes
contract.invokeandcontract.withOptionswithpaymasterDetails, andaccount.executePaymasterTransaction, work again when a parameter is an object, likecairo.uint256(100).paymasterDetails,contract.invokeandcontract.estimatenow send the calldata compiled with the ABI, likeexecute().account.getOutsideTransactionaccepts the same calldata forms asexecute(). In its result,outsideExecution.calls[].calldatais now always written as decimal strings, even when the call gave hex values. The values and the signed hash do not change.Development related changes
__tests__/utils/outsideExecution.test.ts, with 4 tests.npm run test:unitruns it too.__tests__/accountPaymaster.test.ts: a new test sends a call with a u256 object through the paymaster safety check.__tests__/pluginPaymaster.test.ts: two new tests check thatcontract.invokeandcontract.estimatewithpaymasterDetailssend the calldata compiled with the ABI.npm run ts:checkand ESLint pass.__tests__/contractPaymaster.test.tson Sepolia (Pathfinder) passes 4/4. Its two transfer tests failed with 11.4.1. They now send their transactions through the AVNU paymaster.contractPaymaster.test.tsis the only test of theContract+ paymaster path with a real paymaster. It runs only on Testnet, and the CI runs the Testnet tests only formain. This is why the regression was not caught before the 11.4.1 release.Checklist: