Skip to content
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

feat(governance) refactored the script to check multisig signers and settings #13488

Merged
merged 1 commit into from Mar 19, 2024

Conversation

julien51
Copy link
Member

Description

In order to make sure we have everything set-up and cleaned up, I am refactoring the mutlsig check!
Here is the current output:

[Ethereum (1)]: 0x9168EABE624e9515f3836bA1716EC1DDd4C461D4 ❌ Extra signers: 0x2785f2a3DDaCfDE5947F1A9D6c878CCD7F885400,0x7A23608a8eBe71868013BDA0d900351A83bb4Dc2,0x8de33D8204929ceb2F7AA6299d0643a7f6664c9b
[Goerli (5)]: 0x95C06469e557d8645966077891B4aeDe8D55A755 Couldn't fetch multisig info: request to https://safe-transaction-goerli.safe.global/api/v1/safes/0x95C06469e557d8645966077891B4aeDe8D55A755/ failed, reason: getaddrinfo ENOTFOUND safe-transaction-goerli.safe.global
[Optimism (10)]: 0x6E78b4447e34e751EC181DCBed63633aA753e145 ❌ Extra signers: 0x2785f2a3DDaCfDE5947F1A9D6c878CCD7F885400,0x7A23608a8eBe71868013BDA0d900351A83bb4Dc2,0x8de33D8204929ceb2F7AA6299d0643a7f6664c9b
[BNB Chain (56)]: 0x373D7cbc4F2700719DEa237500c7a154310B0F9B ❌ Extra signers: 0x2785f2a3DDaCfDE5947F1A9D6c878CCD7F885400,0x7A23608a8eBe71868013BDA0d900351A83bb4Dc2,0x8de33D8204929ceb2F7AA6299d0643a7f6664c9b
[Gnosis Chain (100)]: 0xfAC611a5b5a578628C28F77cEBDDB8C6159Ae79D ❌ Extra signers: 0x2785f2a3DDaCfDE5947F1A9D6c878CCD7F885400,0x7A23608a8eBe71868013BDA0d900351A83bb4Dc2,0x8de33D8204929ceb2F7AA6299d0643a7f6664c9b
[Polygon (137)]: 0x479f3830fbd715342868BA95E438609BCe443DFB ❌ Extra signers: 0x2785f2a3DDaCfDE5947F1A9D6c878CCD7F885400,0x7A23608a8eBe71868013BDA0d900351A83bb4Dc2,0x8de33D8204929ceb2F7AA6299d0643a7f6664c9b
[zkSync Era (324)]: 0xFa5592CE9C52FA5214ccEa10cB72Faa88eC80a3c ❌ Extra signers: 0x2785f2a3DDaCfDE5947F1A9D6c878CCD7F885400,0x7A23608a8eBe71868013BDA0d900351A83bb4Dc2,0x8de33D8204929ceb2F7AA6299d0643a7f6664c9b
[zkEVM (Polygon) (1101)]: 0xD62EF39c54d9100B17c8fA3C2D15e0262338AED0 2 pending txs are waiting to be signed
[zkEVM (Polygon) (1101)]: 0xD62EF39c54d9100B17c8fA3C2D15e0262338AED0 ❌ Unexpected policy: 2/9 for 4/6 expected
[zkEVM (Polygon) (1101)]: 0xD62EF39c54d9100B17c8fA3C2D15e0262338AED0 ❌ Extra signers: 0x2785f2a3DDaCfDE5947F1A9D6c878CCD7F885400,0x7A23608a8eBe71868013BDA0d900351A83bb4Dc2,0x8de33D8204929ceb2F7AA6299d0643a7f6664c9b
[localhost (31337)]: undefined Couldn't fetch multisig info: There is no transaction service available for chainId 31337. Please set the txServiceUrl property to use a custom transaction service.
[Arbitrum (42161)]: 0x310e9f9E3918a71dB8230cFCF32a083c7D9536d0 1 pending txs are waiting to be signed
[Arbitrum (42161)]: 0x310e9f9E3918a71dB8230cFCF32a083c7D9536d0 ❌ Unexpected policy: 2/10 for 4/6 expected
[Arbitrum (42161)]: 0x310e9f9E3918a71dB8230cFCF32a083c7D9536d0 ❌ Extra signers: 0x2785f2a3DDaCfDE5947F1A9D6c878CCD7F885400,0x7A23608a8eBe71868013BDA0d900351A83bb4Dc2,0x8de33D8204929ceb2F7AA6299d0643a7f6664c9b,0xDD8e2548da5A992A63aE5520C6bC92c37a2Bcc44
[Celo (42220)]: 0xc293E2da9E558bD8B1DFfC4a7b174729fAb2e4E8 1 pending txs are waiting to be signed
[Celo (42220)]: 0xc293E2da9E558bD8B1DFfC4a7b174729fAb2e4E8 ❌ Unexpected policy: 2/9 for 4/6 expected
[Celo (42220)]: 0xc293E2da9E558bD8B1DFfC4a7b174729fAb2e4E8 ❌ Extra signers: 0x2785f2a3DDaCfDE5947F1A9D6c878CCD7F885400,0x7A23608a8eBe71868013BDA0d900351A83bb4Dc2,0x8de33D8204929ceb2F7AA6299d0643a7f6664c9b
[Avalanche (C-Chain) (43114)]: 0xEc7777C51327917fd2170c62873272ea168120Cb 3 pending txs are waiting to be signed
[Avalanche (C-Chain) (43114)]: 0xEc7777C51327917fd2170c62873272ea168120Cb ❌ Unexpected policy: 3/8 for 4/6 expected
[Avalanche (C-Chain) (43114)]: 0xEc7777C51327917fd2170c62873272ea168120Cb ❌ Extra signers: 0x2785f2a3DDaCfDE5947F1A9D6c878CCD7F885400,0x7A23608a8eBe71868013BDA0d900351A83bb4Dc2,0x8de33D8204929ceb2F7AA6299d0643a7f6664c9b
[Avalanche (C-Chain) (43114)]: 0xEc7777C51327917fd2170c62873272ea168120Cb ❌ Missing signers: 0x9d3ea9e9adde71141f4534dB3b9B80dF3D03Ee5f
[Linea (59144)]: undefined Couldn't fetch multisig info: Cannot read properties of undefined (reading 'split')
[Mumbai (Polygon) (80001)]: 0x12E37A8880801E1e5290c815a894d322ac591607 Couldn't fetch multisig info: There is no transaction service available for chainId 80001. Please set the txServiceUrl property to use a custom transaction service.
[Scroll (534352)]: 0x0feE9413A626a05a08AcB0E0e5D6A483e6A0a172 1 pending txs are waiting to be signed
[Scroll (534352)]: 0x0feE9413A626a05a08AcB0E0e5D6A483e6A0a172 ❌ Unexpected policy: 2/8 for 4/6 expected
[Scroll (534352)]: 0x0feE9413A626a05a08AcB0E0e5D6A483e6A0a172 ❌ Extra signers: 0x2785f2a3DDaCfDE5947F1A9D6c878CCD7F885400,0x7A23608a8eBe71868013BDA0d900351A83bb4Dc2,0x8de33D8204929ceb2F7AA6299d0643a7f6664c9b
[Scroll (534352)]: 0x0feE9413A626a05a08AcB0E0e5D6A483e6A0a172 ❌ Missing signers: 0x9d3ea9e9adde71141f4534dB3b9B80dF3D03Ee5f

The Goerli and Mumbai failures are due to these networks being deprecated and we will remove them on our end shortly.
The localhost one will soon go away (as soon as #13321 gets merged).

For zkEVM, the transaction to change the policy to 4 is up. Please don't sign the rejection!

For Celo, the transaction to change the policy to 4 is up.

For Avax, we have the following transactions:

  • protocol upgrade
  • add a signer
  • change the policy from 2 to 4!

For scroll, the transaction to change the policy to 4 is up

Issues

Fixes #
Refs #

Checklist:

  • 1 PR, 1 purpose: my Pull Request applies to a single purpose
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the docs to reflect my changes if applicable
  • I have added tests (and stories for frontend components) that prove my fix is effective or that my feature works
  • I have performed a self-review of my own code
  • If my code involves visual changes, I am adding applicable screenshots to this thread

Release Note Draft Snippet

@julien51 julien51 requested a review from clemsos March 19, 2024 18:29
@cla-bot cla-bot bot added the cla-signed label Mar 19, 2024
@julien51
Copy link
Member Author

Once the configs are good across the board I will issue the transactions to remove the extra signers!

Copy link
Member

@clemsos clemsos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG, maybe just add a dynamic way to get expected signers ?

@@ -5,8 +5,25 @@ const { networks } = require('@unlock-protocol/networks')

const SafeApiKit = require('@safe-global/api-kit').default

const getMultigiInfo = async (chainId, multisig) => {
const prodSigners = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we get these from mainnet directly ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering about that, but then realized it was going to be weird to check something against itself...

'0x246A13358Fb27523642D86367a51C2aEB137Ac6C', // cr
].sort()

const devSigners = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idem maybe use Sepolia as reference ?

@julien51 julien51 merged commit 588de46 into master Mar 19, 2024
12 checks passed
@julien51 julien51 deleted the multisig-hygiene branch March 19, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants