Skip to content

Commit

Permalink
Fix Passkey Verification (#486)
Browse files Browse the repository at this point in the history
In deploying the Passkey v0.2.1 contracts, I noticed that the local
verification script wasn't working as well as Etherscan verification of
the FCL verifier contract. This PR fixes both of these things and is
related to a few issues:

1. The `solc` package version was not correctly specified for each
`localVerify` script. It was pulling in a `solc` version based on
transient dependencies which doesn't necessarily match what the
contracts are compiled with (and thus affects local verification). This
was an issue shared by all Solidity packages.
2. Local verification was not correctly building the Solidity compiler
input JSON. It turns out that the way we build compiler input would
sometimes only pass in the Keccak-256 hash of the source instead of the
actual Solidity source - so we add an additional condition in the task
to read the source from disk if necessary (this is a `hardhat-deploy`
inconsistency).
3. The `etherscan-verify` task from the `hardhat-deploy` package does
not work for contracts with Solidity compiler settings overrides such as
the FCL P-256 verifier contract (which has different compiler settings
from the rest of the contracts). We have a special manual
`hardhat-verify` plugin call in the `deploy-all` task to work around the
issue.
  • Loading branch information
nlordell authored Aug 20, 2024
1 parent b6bd0d0 commit dfd3b05
Show file tree
Hide file tree
Showing 14 changed files with 355 additions and 303 deletions.
5 changes: 2 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,14 @@ coverage/
coverage.json
deployments/
dist/
typechain-types
typechain-types/
venv/
*.log
.env


.DS_Store
.zos.session
env/
.env
.history
coverage*
.certora_internal
Expand Down
2 changes: 1 addition & 1 deletion modules/4337/certora/conf/Safe4337Module.conf
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@
"verify": "Safe4337Module:certora/specs/Safe4337Module.spec",
"packages": [
"@account-abstraction=../../node_modules/.pnpm/@[email protected]/node_modules/@account-abstraction",
"@safe-global=../../node_modules/.pnpm/@[email protected][email protected][email protected]_utf-8-validate@5.0.10_/node_modules/@safe-global"
"@safe-global=../../node_modules/.pnpm/@[email protected][email protected][email protected]_utf-8-validate@6.0.4_/node_modules/@safe-global"
]
}
2 changes: 1 addition & 1 deletion modules/4337/certora/conf/SignatureLengthCheck.conf
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@
"verify": "Safe4337ModuleHarness:certora/specs/SignatureLengthCheck.spec",
"packages": [
"@account-abstraction=../../node_modules/.pnpm/@[email protected]/node_modules/@account-abstraction",
"@safe-global=../../node_modules/.pnpm/@[email protected][email protected][email protected]_utf-8-validate@5.0.10_/node_modules/@safe-global"
"@safe-global=../../node_modules/.pnpm/@[email protected][email protected][email protected]_utf-8-validate@6.0.4_/node_modules/@safe-global"
]
}
2 changes: 1 addition & 1 deletion modules/4337/certora/conf/TransactionExecutionMethods.conf
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@
"verify": "Safe4337Module:certora/specs/TransactionExecutionMethods.spec",
"packages": [
"@account-abstraction=../../node_modules/.pnpm/@[email protected]/node_modules/@account-abstraction",
"@safe-global=../../node_modules/.pnpm/@[email protected][email protected][email protected]_utf-8-validate@5.0.10_/node_modules/@safe-global"
"@safe-global=../../node_modules/.pnpm/@[email protected][email protected][email protected]_utf-8-validate@6.0.4_/node_modules/@safe-global"
]
}
2 changes: 1 addition & 1 deletion modules/4337/certora/conf/ValidationDataLastBitOne.conf
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@
"verify": "Safe4337Module:certora/specs/ValidationDataLastBitOne.spec",
"packages": [
"@account-abstraction=../../node_modules/.pnpm/@[email protected]/node_modules/@account-abstraction",
"@safe-global=../../node_modules/.pnpm/@[email protected][email protected][email protected]_utf-8-validate@5.0.10_/node_modules/@safe-global"
"@safe-global=../../node_modules/.pnpm/@[email protected][email protected][email protected]_utf-8-validate@6.0.4_/node_modules/@safe-global"
]
}
2 changes: 1 addition & 1 deletion modules/4337/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"hardhat": "^2.22.5",
"hardhat-deploy": "^0.12.4",
"husky": "^9.0.11",
"solc": "^0.8.25",
"solc": "0.8.23",
"solhint": "^5.0.1",
"ts-node": "^10.9.2",
"typescript": "^5.5.2",
Expand Down
8 changes: 6 additions & 2 deletions modules/4337/src/tasks/localVerify.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { promises as fs } from 'node:fs'
import { task } from 'hardhat/config'
import { loadSolc } from '../utils/solc'

Expand All @@ -15,11 +16,14 @@ task('local-verify', 'Verifies that the local deployment files correspond to the
delete meta.compiler
delete meta.output
delete meta.version
const sources = Object.values<Record<string, unknown>>(meta.sources)
for (const source of sources) {
const sources = Object.entries<Record<string, unknown>>(meta.sources)
for (const [filename, source] of sources) {
for (const key of Object.keys(source)) {
if (allowedSourceKey.indexOf(key) < 0) delete source[key]
}
if (source['content'] === undefined) {
source['content'] = await fs.readFile(filename, 'utf-8')
}
}
meta.settings.outputSelection = {}
const targets = Object.entries<string>(meta.settings.compilationTarget)
Expand Down
1 change: 1 addition & 0 deletions modules/passkey/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"ethers": "^6.13.1",
"hardhat": "^2.22.5",
"hardhat-deploy": "^0.12.4",
"solc": "0.8.26",
"solhint": "^5.0.1",
"ts-node": "^10.9.2",
"typescript": "^5.5.2"
Expand Down
9 changes: 9 additions & 0 deletions modules/passkey/src/tasks/deployContracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,15 @@ import { task } from 'hardhat/config'
task('deploy-contracts', 'Deploys and verifies Safe contracts').setAction(async (_, { deployments, run }) => {
await run('deploy')
await run('local-verify')

// Unfortunately, the `etherscan-verify` task from the `hardhat-deploy` package cannot deal with
// contracts compiled with different Solidity settings in the same project :/. We work around this
// by first manually verifying the FCL P-256 verifier contract (which is the only contract that is
// build with special Solidity settings) so that it is already verified and does not fail in the
// `etherscan-verify` step below.
const { address: fclP256Verifier } = await deployments.get('FCLP256Verifier')
await run('verify', { address: fclP256Verifier, contract: 'contracts/verifiers/FCLP256Verifier.sol:FCLP256Verifier' })

await run('etherscan-verify', { forceLicense: true, license: 'LGPL-3.0' })
await run('sourcify')

Expand Down
8 changes: 6 additions & 2 deletions modules/passkey/src/tasks/localVerify.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { promises as fs } from 'node:fs'
import { task } from 'hardhat/config'
import { loadSolc } from '../utils/solc'

Expand All @@ -15,11 +16,14 @@ task('local-verify', 'Verifies that the local deployment files correspond to the
delete meta.compiler
delete meta.output
delete meta.version
const sources = Object.values<Record<string, unknown>>(meta.sources)
for (const source of sources) {
const sources = Object.entries<Record<string, unknown>>(meta.sources)
for (const [filename, source] of sources) {
for (const key of Object.keys(source)) {
if (allowedSourceKey.indexOf(key) < 0) delete source[key]
}
if (source['content'] === undefined) {
source['content'] = await fs.readFile(filename, 'utf-8')
}
}
meta.settings.outputSelection = {}
const targets = Object.entries<string>(meta.settings.compilationTarget)
Expand Down
1 change: 1 addition & 0 deletions modules/recovery/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
"ethers": "^6.13.1",
"hardhat": "^2.22.5",
"hardhat-deploy": "^0.12.4",
"solc": "0.8.20",
"typescript": "^5.5.2",
"yargs": "^17.7.2"
},
Expand Down
8 changes: 6 additions & 2 deletions modules/recovery/src/tasks/localVerify.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { promises as fs } from 'node:fs'
import { task } from 'hardhat/config'
import { loadSolc } from '../utils/solc'

Expand All @@ -15,11 +16,14 @@ task('local-verify', 'Verifies that the local deployment files correspond to the
delete meta.compiler
delete meta.output
delete meta.version
const sources = Object.values<Record<string, unknown>>(meta.sources)
for (const source of sources) {
const sources = Object.entries<Record<string, unknown>>(meta.sources)
for (const [filename, source] of sources) {
for (const key of Object.keys(source)) {
if (allowedSourceKey.indexOf(key) < 0) delete source[key]
}
if (source['content'] === undefined) {
source['content'] = await fs.readFile(filename, 'utf-8')
}
}
meta.settings.outputSelection = {}
const targets = Object.entries<string>(meta.settings.compilationTarget)
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"scripts": {
"fmt:global": "prettier --write .",
"fmt:global-check": "prettier --check .",
"lint:monorepo": "sherif -i @openzeppelin/contracts -r root-package-manager-field"
"lint:monorepo": "sherif -i @openzeppelin/contracts -i solc -r root-package-manager-field"
},
"repository": {
"type": "git",
Expand Down
Loading

0 comments on commit dfd3b05

Please sign in to comment.