-
Notifications
You must be signed in to change notification settings - Fork 992
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
Fixes for 1.0.0 #90
Fixes for 1.0.0 #90
Conversation
rmeissner
commented
Feb 22, 2019
- Implemented critical proposals from https://github.com/runtimeverification/verified-smart-contracts/blob/master/gnosis/GnosisSafe_RuntimeVerification.pdf
- Adjusted EIP-1271 interface
- Cleanup of naming
//); | ||
bytes32 public constant SAFE_TX_TYPEHASH = 0x14d461bc7412367e924637b363c7bf29b8f47e2f84869f4426e5633d8af47b20; | ||
bytes32 public constant SAFE_TX_TYPEHASH = 0xbb8310d486368db6bd6f849402fdd73ad53d316b5a4b2644ad6efe0f941286d8; |
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.
@Uxio0 this will require changes in the clients, so we need to add a way that the clients know which version to use for signing (maybe return the contract version with the estimates)
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.
Alternatively we could stick to dataGas (but imo this is a really bad name ... on the other side the user will not see it)
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.
I would prefer that clients query the GET /safes/{address}/
endpoint, I don't like how the estimate endpoint is growing. Currently masterCopy
, nonce
, threshold
and owners
are returned, I will add version
to that endpoint too. Would be ok for you @rmeissner?
* Fix SafeTransactionDataPartial import * fix documentation: wrong import (safe-global#87) * fix import Co-authored-by: Germán Martínez <germartinez@users.noreply.github.com> Co-authored-by: Parv Garg <parv3213@gmail.com>
### 🕓 Changelog Safe multisig versions `< 1.0.0` use a legacy (i.e. the parameter value `baseGas` was called `dataGas` previously) [`SAFE_TX_TYPEHASH`](https://github.com/safe-global/safe-smart-account/blob/427d6f7e779431333c54bcb4d4cde31e4d57ce96/contracts/GnosisSafe.sol#L25-L28) value: ```solidity //keccak256( // "SafeTx(address to,uint256 value,bytes data,uint8 operation,uint256 safeTxGas,uint256 dataGas,uint256 gasPrice,address gasToken,address refundReceiver,uint256 nonce)" //); bytes32 public constant SAFE_TX_TYPEHASH = 0x14d461bc7412367e924637b363c7bf29b8f47e2f84869f4426e5633d8af47b20; ``` Starting with version [`1.0.0`](https://github.com/safe-global/safe-smart-account/releases/tag/v1.0.0), `baseGas` was introduced: safe-global/safe-smart-account#90. This PR introduces additional logic to the script to accommodate the breaking change in Safe versions. Furthermore, all Safe versions `< 0.1.0` are not supported and the script issues as warning to the user. ### Testing **Ethereum:** The Safe multisig [`0xecd11858a4bcc35A51084Ebe672beaCe01142fcA`](https://etherscan.io/address/0xecd11858a4bcc35A51084Ebe672beaCe01142fcA) is based on version [`0.1.0`](https://github.com/safe-global/safe-smart-account/releases/tag/v0.1.0). Run: ```console ./safe_hashes.sh --network ethereum --address 0xecd11858a4bcc35A51084Ebe672beaCe01142fcA --nonce 61 ``` ```console > Hashes: Domain hash: 0x4181D94EE9F43DC08D31A2C86EB37B704F2051787A9C84D1397A60D07136D1C1 Message hash: 0xA6CF82FF9752EACE8FA8389C19C6B5FA8706AE702D7CF07F7B6734E73BFCD27D Safe transaction hash: 0x85a7e913bba17df41ff87ce425bdf950ad8fa02343ec1c7652a823c1b2a9dab5 ``` To verify the domain hash, you can use `cast`: ```console cast call 0xecd11858a4bcc35A51084Ebe672beaCe01142fcA "domainSeparator()" -r https://eth.llamarpc.com ``` which will output: ```console 0x4181d94ee9f43dc08d31a2c86eb37b704f2051787a9c84d1397a60d07136d1c1 ``` For the message hash and the Safe transaction hash, you can use the `evaluate` feature of Tenderly in one of the multisig transactions: https://dashboard.tenderly.co/tx/mainnet/0xc139e324bc231c5f7b8a9a27e775295118e9cbc06995f89225261abd0420f362/debugger?trace=0.1.1.0  https://dashboard.tenderly.co/tx/mainnet/0xc139e324bc231c5f7b8a9a27e775295118e9cbc06995f89225261abd0420f362/debugger?trace=0.1.2  --------- Signed-off-by: Pascal Marco Caversaccio <pascal.caversaccio@hotmail.ch>
### 🕓 Changelog Safe multisig versions `< 1.0.0` use a legacy (i.e. the parameter value `baseGas` was called `dataGas` previously) [`SAFE_TX_TYPEHASH`](https://github.com/safe-global/safe-smart-account/blob/427d6f7e779431333c54bcb4d4cde31e4d57ce96/contracts/GnosisSafe.sol#L25-L28) value: ```solidity //keccak256( // "SafeTx(address to,uint256 value,bytes data,uint8 operation,uint256 safeTxGas,uint256 dataGas,uint256 gasPrice,address gasToken,address refundReceiver,uint256 nonce)" //); bytes32 public constant SAFE_TX_TYPEHASH = 0x14d461bc7412367e924637b363c7bf29b8f47e2f84869f4426e5633d8af47b20; ``` Starting with version [`1.0.0`](https://github.com/safe-global/safe-smart-account/releases/tag/v1.0.0), `baseGas` was introduced: safe-global/safe-smart-account#90. This PR introduces additional logic to the script to accommodate the breaking change in Safe versions. Furthermore, all Safe versions `< 0.1.0` are not supported and the script issues as warning to the user. ### Testing **Ethereum:** The Safe multisig [`0xecd11858a4bcc35A51084Ebe672beaCe01142fcA`](https://etherscan.io/address/0xecd11858a4bcc35A51084Ebe672beaCe01142fcA) is based on version [`0.1.0`](https://github.com/safe-global/safe-smart-account/releases/tag/v0.1.0). Run: ```console ./safe_hashes.sh --network ethereum --address 0xecd11858a4bcc35A51084Ebe672beaCe01142fcA --nonce 61 ``` ```console > Hashes: Domain hash: 0x4181D94EE9F43DC08D31A2C86EB37B704F2051787A9C84D1397A60D07136D1C1 Message hash: 0xA6CF82FF9752EACE8FA8389C19C6B5FA8706AE702D7CF07F7B6734E73BFCD27D Safe transaction hash: 0x85a7e913bba17df41ff87ce425bdf950ad8fa02343ec1c7652a823c1b2a9dab5 ``` To verify the domain hash, you can use `cast`: ```console cast call 0xecd11858a4bcc35A51084Ebe672beaCe01142fcA "domainSeparator()" -r https://eth.llamarpc.com ``` which will output: ```console 0x4181d94ee9f43dc08d31a2c86eb37b704f2051787a9c84d1397a60d07136d1c1 ``` For the message hash and the Safe transaction hash, you can use the `evaluate` feature of Tenderly in one of the multisig transactions: https://dashboard.tenderly.co/tx/mainnet/0xc139e324bc231c5f7b8a9a27e775295118e9cbc06995f89225261abd0420f362/debugger?trace=0.1.1.0  https://dashboard.tenderly.co/tx/mainnet/0xc139e324bc231c5f7b8a9a27e775295118e9cbc06995f89225261abd0420f362/debugger?trace=0.1.2  --------- Signed-off-by: Pascal Marco Caversaccio <pascal.caversaccio@hotmail.ch>