-
Notifications
You must be signed in to change notification settings - Fork 43
Update gas station SDK with post audit contracts #1089
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
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 9997e60:
|
… for post-audit contract
andrewkmin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double checking a few things:
want to confirm that these arguments / param names are still correct:
sdk/packages/gas-station/src/policyUtils.ts
Lines 272 to 274 in 632d53c
| // Check output contract address using ABI parsing | |
| // Turnkey parses execute(address _targetEoA, address _to, uint256 ethAmount, bytes _data) | |
| // and exposes arguments via eth.tx.contract_call_args |
sdk/packages/gas-station/src/policyUtils.ts
Lines 201 to 205 in 632d53c
| * "(eth.tx.contract_call_args['_to'] == '0x833...usdc' || " + | |
| * "eth.tx.contract_call_args['_to'] == '0x6b1...dai') && " + | |
| * "(eth.tx.contract_call_args['_targetEoA'] == '0xali...ce' || " + | |
| * "eth.tx.contract_call_args['_targetEoA'] == '0xbob...by') && " + | |
| * "eth.tx.contract_call_args['ethAmount'] <= 100000000000000000 && " + |
very small nit: I see that several examples contain dummy IDs like the following:
sdk/packages/gas-station/src/policyUtils.ts
Lines 51 to 53 in 632d53c
| * organizationId: "org-123", | |
| * eoaUserId: "user-456", | |
| * additionalApprovers: ["backup-user-789"], |
I'd recommend altering these such that it's more clear that these are UUIDs, not tags/simple strings
| @@ -1,7 +1,5 @@ | |||
| # Turnkey Gas Station SDK | |||
|
|
|||
| > **⚠️ BETA WARNING**: This SDK is currently in beta. The underlying smart contracts are **unaudited** and should not be used in production environments. Use at your own risk. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹
| type: "function", | ||
| }, | ||
| { | ||
| inputs: [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we make explicit note of this somewhere? or no need given this wasn't really being used in the first place?
… new contract addresses and EIP-712 field name adjustments. Modify related documentation and examples for consistency.
Summary & Motivation
Updated the
@turnkey/gas-stationSDK to align with the audited smart contract changes. The audit resulted in several interface updates:Contract Changes:
to,value,data) instead of the previous descriptive names (outputContract,ethAmount,arguments)SDK Updates:
DEFAULT_EXECUTION_CONTRACTaddress from0x4ece92b06C7d2d99d87f052E0Fca47Fb180c3348to0x00000000008c57a1CE37836a5e9d36759D070d8cDEFAULT_DELEGATE_CONTRACTaddress from0xC2a37Ee08cAc3778d9d05FF0a93FD5B553C77E3ato0x000066a00056CD44008768E2aF00696e19A30084Executiontypehash field names to match the contract's canonical interfaceApproveThenExecutetypehash field names to match the contract's canonical interfacebuildIntentSigningPolicyto reference the new field names (to,valueinstead ofoutputContract,ethAmount)Files Modified:
packages/gas-station/src/config.ts- Updated contract addressespackages/gas-station/src/intentBuilder.ts- Updated EIP-712 type definitions and message objectspackages/gas-station/src/policyUtils.ts- Updated policy condition field references and documentationHow I Tested These Changes
Unit Tests:
Ran the comprehensive policy enforcement test suite (
npm run test:policies) which validates:Layer 1: EOA Intent Signing Policy
Layer 2: Paymaster Execution Policy
Layer 3: Multi-Approval Consensus
Result: All 7 tests passed (28.9s runtime), confirming:
eth.eip_712.message['to']andeth.eip_712.message['value']fieldsDid you add a changeset?
yes