search

safe-global / safe-smart-account

Safe allows secure management of blockchain assets.
https://safe.global
GNU Lesser General Public License v3.0
1.84k stars 907 forks source link

LocalVerify Failures #689

Closed JABirchall closed 11 months ago

JABirchall commented 11 months ago

Prerequisites

Add custom network node URL Follow custom chain deployment npm run deploy-all custom

Description

I was following instructions to deploy safe-contracts on a custom network, and after running the commands, 2 contracts failed local verify.

> @gnosis.pm/safe-contracts@1.3.0 deploy-all
> hardhat deploy-contracts --network custom

Nothing to compile
reusing "SimulateTxAccessor" at 0x59AD6735bCd8152B84860Cb256dD9e96b85F69Da
reusing "GnosisSafeProxyFactory" at 0xa6B71E26C5e0845f74c812102Ca7114b6a896AB2
reusing "DefaultCallbackHandler" at 0x1AC114C2099aFAf5261731655Dc6c306bFcd4Dbd
reusing "CompatibilityFallbackHandler" at 0xf48f2B2d2a534e402487b3ee7C18c33Aec0Fe5e4
reusing "CreateCall" at 0x7cbB62EaA69F79e6873cD1ecB2392971036cFAa4
reusing "MultiSend" at 0xA238CBeb142c10Ef7Ad8442C6D1f9E89e07e7761
reusing "MultiSendCallOnly" at 0x40A2aCCbd92BCA938b02010E17A5b8929b49130D
reusing "SignMessageLib" at 0xA65387F16B013cf2Af4605Ad8aA5ec25a2cbA3a2
reusing "GnosisSafeL2" at 0x3E5c63644E683549055b9Be8653de26E0B4CD36E
reusing "GnosisSafe" at 0xd9Db270c1B5E3Bd161E8c8503c55cEABeE709552
Verification status for CompatibilityFallbackHandler: SUCCESS
Verification status for CreateCall: SUCCESS
Verification status for DefaultCallbackHandler: SUCCESS
Verification status for GnosisSafe: SUCCESS
Verification status for GnosisSafeL2: SUCCESS
Verification status for GnosisSafeProxyFactory: SUCCESS
Verification status for MultiSend: FAILURE
Verification status for MultiSendCallOnly: SUCCESS
Verification status for SignMessageLib: SUCCESS
Verification status for SimulateTxAccessor: FAILURE
Network with chainId: 622277 not supported

Notice how SimulateTxAccessor and MultiSend failed local verify. The addresses seems to be correct, but the bytecode verification fails.

Environment

Steps to reproduce

Add custom network node URL Follow custom chain deployment npm run deploy-all custom

Additional context

I dont know if this is a major issue or something, or if there is a problem with the deployment. But I thought it might be worth raising

nlordell commented 11 months ago

Thanks for reporting the issue. AFAIU, this is unfortunately somewhat expected for that contract. Specifically, the contract makes use of immutable which are values that get stored in the contract code at init time:

https://github.com/safe-global/safe-contracts/blob/b344bd866b99f8f51fae20ea2b98680d5219028d/contracts/accessors/SimulateTxAccessor.sol#L12C5-L12C49

This means that the deployedBytecode for the contract artefact will incorrectly have push32 0 placeholders where the immutable gets used instead of the actual value in the code that is being read from the RPC node that it is comparing to.

The correct fix would be to correctly mask the immutable placeholders from the retrieved contract byte code before hashing and comparing.

With that in mind, this is a valid issue that should be fixed in the local-verify script, but I do believe it is safe to ignore.

JABirchall commented 11 months ago

With that in mind, this is a valid issue that should be fixed in the local-verify script, but I do believe it is safe to ignore.

Cool, as long as its something ive not done wrong :P