-
Notifications
You must be signed in to change notification settings - Fork 310
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: add key registry to deployment (e2e & sandbox) #5875
Changes from 13 commits
3eb0d7b
72ca505
3da10c1
1f166da
f09c76b
dfde46d
7ead539
9de1a5f
eb6e7ae
8a26311
67e71c9
3f4fe51
1366fc9
4a8eca9
437514f
99a5b2b
be3f86c
5708964
ff5af43
c6b1b7e
2119e01
d46db22
c3bbd71
b809aa2
d210838
4e2f5a4
501e079
426be8b
33d2e61
a2f858b
de746ea
41e6ec5
3e8d86b
4355e8f
59e5cfa
bf2f5fa
a87bbf5
7a69d69
3484dde
06f7604
fecb707
40fb063
aa6bde4
12ca58c
3922eef
693137b
f3f2fc4
3b9db8e
81c3496
2e75e61
a5e1b96
946a835
2e8ed6c
8e97be0
715cfed
8365d54
73853cf
abe64eb
55f1006
5cb8d5f
af07727
ea63152
6e35053
666921d
ecdcf29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,7 @@ import { | |
RollupBytecode, | ||
} from '@aztec/l1-artifacts'; | ||
import { getCanonicalGasToken } from '@aztec/protocol-contracts/gas-token'; | ||
import { getCanonicalKeyRegistry } from '@aztec/protocol-contracts/key-registry'; | ||
import { type PXEServiceConfig, createPXEService, getPXEServiceConfig } from '@aztec/pxe'; | ||
|
||
import { | ||
|
@@ -182,6 +183,26 @@ async function deployCanonicalL2GasToken(deployer: Wallet, l1ContractAddresses: | |
logger.info(`Deployed Gas Token on L2 at ${canonicalGasToken.address}`); | ||
} | ||
|
||
/** | ||
* Deploys the key registry on L2. | ||
*/ | ||
async function deployCanonicalKeyRegistry(deployer: Wallet) { | ||
const canonicalKeyRegistry = getCanonicalKeyRegistry(); | ||
|
||
if (await deployer.isContractClassPubliclyRegistered(canonicalKeyRegistry.contractClass.id)) { | ||
return; | ||
} | ||
|
||
await new BatchCall(deployer, [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If rebasing you should see the deployment flow that we are normally using for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in |
||
(await registerContractClass(deployer, canonicalKeyRegistry.artifact)).request(), | ||
deployInstance(deployer, canonicalKeyRegistry.instance).request(), | ||
]) | ||
.send() | ||
.wait(); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please throw here an error if the address of the deployed contract does not match the constant. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in |
||
logger.info(`Deployed Key Registry on L2 at ${canonicalKeyRegistry.address}`); | ||
} | ||
|
||
/** Sandbox settings. */ | ||
export type SandboxConfig = AztecNodeConfig & { | ||
/** Mnemonic used to derive the L1 deployer private key.*/ | ||
|
@@ -210,6 +231,10 @@ export async function createSandbox(config: Partial<SandboxConfig> = {}) { | |
const node = await createAztecNode(aztecNodeConfig); | ||
const pxe = await createAztecPXE(node); | ||
|
||
await deployCanonicalKeyRegistry( | ||
new SignerlessWallet(pxe, new DefaultMultiCallEntrypoint(aztecNodeConfig.chainId, aztecNodeConfig.version)), | ||
); | ||
|
||
if (config.enableGas) { | ||
await deployCanonicalL2GasToken( | ||
new SignerlessWallet(pxe, new DefaultMultiCallEntrypoint(aztecNodeConfig.chainId, aztecNodeConfig.version)), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ describe('e2e_voting_contract', () => { | |
votingContract = await EasyPrivateVotingContract.deploy(wallet, owner).send().deployed(); | ||
|
||
logger.info(`Counter contract deployed at ${votingContract.address}`); | ||
}, 25_000); | ||
}, 45_000); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is going in a good direction 🤣 |
||
|
||
afterAll(() => teardown()); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,6 +47,7 @@ import { | |
RollupBytecode, | ||
} from '@aztec/l1-artifacts'; | ||
import { getCanonicalGasToken, getCanonicalGasTokenAddress } from '@aztec/protocol-contracts/gas-token'; | ||
import { getCanonicalKeyRegistry } from '@aztec/protocol-contracts/key-registry'; | ||
import { PXEService, type PXEServiceConfig, createPXEService, getPXEServiceConfig } from '@aztec/pxe'; | ||
import { type SequencerClient } from '@aztec/sequencer-client'; | ||
|
||
|
@@ -260,10 +261,14 @@ async function setupWithRemoteEnvironment( | |
const cheatCodes = CheatCodes.create(config.rpcUrl, pxeClient!); | ||
const teardown = () => Promise.resolve(); | ||
|
||
const { chainId, protocolVersion } = await pxeClient.getNodeInfo(); | ||
// this contract might already have been deployed | ||
// the following deployin functions are idempotent | ||
await deployCanonicalKeyRegistry( | ||
new SignerlessWallet(pxeClient, new DefaultMultiCallEntrypoint(chainId, protocolVersion)), | ||
); | ||
|
||
if (enableGas) { | ||
const { chainId, protocolVersion } = await pxeClient.getNodeInfo(); | ||
// this contract might already have been deployed | ||
// the following function is idempotent | ||
await deployCanonicalGasToken( | ||
new SignerlessWallet(pxeClient, new DefaultMultiCallEntrypoint(chainId, protocolVersion)), | ||
); | ||
|
@@ -397,6 +402,11 @@ export async function setup( | |
logger.verbose('Creating a pxe...'); | ||
const { pxe, wallets } = await setupPXEService(numberOfAccounts, aztecNode!, pxeOpts, logger); | ||
|
||
logger.verbose('Deploying key registry...'); | ||
await deployCanonicalKeyRegistry( | ||
new SignerlessWallet(pxe, new DefaultMultiCallEntrypoint(config.chainId, config.version)), | ||
); | ||
|
||
if (enableGas) { | ||
logger.verbose('Deploying gas token...'); | ||
await deployCanonicalGasToken( | ||
|
@@ -587,3 +597,21 @@ export async function deployCanonicalGasToken(deployer: Wallet) { | |
await expect(deployer.isContractClassPubliclyRegistered(canonicalGasToken.contractClass.id)).resolves.toBe(true); | ||
await expect(deployer.getContractInstance(canonicalGasToken.instance.address)).resolves.toBeDefined(); | ||
} | ||
|
||
export async function deployCanonicalKeyRegistry(deployer: Wallet) { | ||
const canonicalKeyRegistry = getCanonicalKeyRegistry(); | ||
|
||
if (await deployer.isContractClassPubliclyRegistered(canonicalKeyRegistry.contractClass.id)) { | ||
return; | ||
} | ||
|
||
await new BatchCall(deployer, [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in |
||
(await registerContractClass(deployer, canonicalKeyRegistry.artifact)).request(), | ||
deployInstance(deployer, canonicalKeyRegistry.instance).request(), | ||
]) | ||
.send() | ||
.wait(); | ||
|
||
await expect(deployer.isContractClassPubliclyRegistered(canonicalKeyRegistry.contractClass.id)).resolves.toBe(true); | ||
await expect(deployer.getContractInstance(canonicalKeyRegistry.instance.address)).resolves.toBeDefined(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import { loadContractArtifact } from '@aztec/types/abi'; | ||
import { type NoirCompiledContract } from '@aztec/types/noir'; | ||
|
||
import KeyRegistryJson from '../artifacts/KeyRegistry.json' assert { type: 'json' }; | ||
|
||
export const KeyRegistryArtifact = loadContractArtifact(KeyRegistryJson as NoirCompiledContract); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import { computeContractAddressFromInstance, getContractClassFromArtifact } from '@aztec/circuits.js'; | ||
|
||
import { getCanonicalKeyRegistry } from './index.js'; | ||
|
||
describe('KeyRegistry', () => { | ||
it('returns canonical protocol contract', () => { | ||
const contract = getCanonicalKeyRegistry(); | ||
expect(computeContractAddressFromInstance(contract.instance)).toEqual(contract.address); | ||
expect(getContractClassFromArtifact(contract.artifact).id).toEqual(contract.contractClass.id); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a test checking that it matches what we have in the constants as well. OTherwise could be massive pain to figure out 👀 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's better to check it during deployment. It's harder to accidentally miss as it blows all the e2e tests as well. Added a comment above. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've put it in both places for now. |
||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import { type AztecAddress } from '@aztec/circuits.js'; | ||
LHerskind marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
import { type ProtocolContract, getCanonicalProtocolContract } from '../protocol_contract.js'; | ||
import { KeyRegistryArtifact } from './artifact.js'; | ||
|
||
/** Returns the canonical deployment of the gas token. */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment here is wrong 👀 wrong contract name There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah good look, thanks ! Addressed in |
||
export function getCanonicalKeyRegistry(): ProtocolContract { | ||
return getCanonicalProtocolContract(KeyRegistryArtifact, 1337); | ||
} | ||
|
||
export function getCanonicalKeyRegistryAddress(): AztecAddress { | ||
return getCanonicalKeyRegistry().address; | ||
} |
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.
This check only checks contract class so I think it could happen that someone could accidentally deploy it to a different address (for example by using a different salt) and then this would not deploy an instance even though it should.
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.
Addressed in
99a5b2b