-
Notifications
You must be signed in to change notification settings - Fork 284
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
fix: reprocess notes in pxe when a new contract is added #3867
Merged
just-mitch
merged 9 commits into
master
from
2709-reprocess-notes-in-pxe-when-a-new-contract-is-added
Jan 9, 2024
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
742f113
utilize the RunningPromise for managing synchronizer loop
8cd9031
add test demonstrating the bug
f1bccd5
detect the case where the contract does not exist
606863b
store deferred notes in the pxe database
164fe25
simple test passes
227bc11
remove deferred notes from pxe db
d1f7267
cleanup comments, implementation for in-memory database
0c18d84
address PR feedback
3dd9535
start the pxe job queue in the constructor
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import { AztecAddress, MembershipWitness, VK_TREE_HEIGHT } from '@aztec/circuits.js'; | ||
import { FunctionDebugMetadata, FunctionSelector, getFunctionDebugMetadata } from '@aztec/foundation/abi'; | ||
import { ContractNotFoundError } from '@aztec/acir-simulator'; | ||
import { AztecAddress, ContractFunctionDao, MembershipWitness, VK_TREE_HEIGHT } from '@aztec/circuits.js'; | ||
import { FunctionDebugMetadata, FunctionSelector } from '@aztec/foundation/abi'; | ||
import { ContractDatabase, StateInfoProvider } from '@aztec/types'; | ||
|
||
import { ContractTree } from '../contract_tree/index.js'; | ||
|
@@ -47,20 +48,25 @@ export class ContractDataOracle { | |
/** | ||
* Retrieves the artifact of a specified function within a given contract. | ||
* The function is identified by its name, which is unique within a contract. | ||
* Throws if the contract has not been added to the database. | ||
* | ||
* @param contractAddress - The AztecAddress representing the contract containing the function. | ||
* @param functionName - The name of the function. | ||
* @returns The corresponding function's artifact as an object. | ||
* @returns The corresponding function's artifact as an object | ||
*/ | ||
public async getFunctionArtifactByName(contractAddress: AztecAddress, functionName: string) { | ||
const contract = await this.db.getContract(contractAddress); | ||
return contract?.functions.find(f => f.name === functionName); | ||
public async getFunctionArtifactByName( | ||
contractAddress: AztecAddress, | ||
functionName: string, | ||
): Promise<ContractFunctionDao | undefined> { | ||
const tree = await this.getTree(contractAddress); | ||
return tree.contract.getFunctionArtifactByName(functionName); | ||
Comment on lines
+61
to
+62
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. Was this changed so it would warm up the |
||
} | ||
|
||
/** | ||
* Retrieves the debug metadata of a specified function within a given contract. | ||
* The function is identified by its selector, which is a unique code generated from the function's signature. | ||
* Returns undefined if the debug metadata for the given function is not found. | ||
* Throws if the contract has not been added to the database. | ||
* | ||
* @param contractAddress - The AztecAddress representing the contract containing the function. | ||
* @param selector - The function selector. | ||
|
@@ -70,14 +76,14 @@ export class ContractDataOracle { | |
contractAddress: AztecAddress, | ||
selector: FunctionSelector, | ||
): Promise<FunctionDebugMetadata | undefined> { | ||
const contract = await this.db.getContract(contractAddress); | ||
const functionArtifact = contract?.functions.find(f => f.selector.equals(selector)); | ||
const tree = await this.getTree(contractAddress); | ||
const functionArtifact = tree.contract.getFunctionArtifact(selector); | ||
|
||
if (!contract || !functionArtifact) { | ||
if (!functionArtifact) { | ||
return undefined; | ||
} | ||
|
||
return getFunctionDebugMetadata(contract, functionArtifact.name); | ||
return tree.contract.getFunctionDebugMetadataByName(functionArtifact.name); | ||
} | ||
|
||
/** | ||
|
@@ -88,6 +94,7 @@ export class ContractDataOracle { | |
* @param contractAddress - The contract's address. | ||
* @param selector - The function selector. | ||
* @returns A Promise that resolves to a Buffer containing the bytecode of the specified function. | ||
* @throws Error if the contract address is unknown or not found. | ||
*/ | ||
public async getBytecode(contractAddress: AztecAddress, selector: FunctionSelector) { | ||
const tree = await this.getTree(contractAddress); | ||
|
@@ -147,12 +154,12 @@ export class ContractDataOracle { | |
* @returns A ContractTree instance associated with the specified contract address. | ||
* @throws An Error if the contract is not found in the ContractDatabase. | ||
*/ | ||
private async getTree(contractAddress: AztecAddress) { | ||
private async getTree(contractAddress: AztecAddress): Promise<ContractTree> { | ||
let tree = this.trees.find(t => t.contract.completeAddress.address.equals(contractAddress)); | ||
if (!tree) { | ||
const contract = await this.db.getContract(contractAddress); | ||
if (!contract) { | ||
throw new Error(`Unknown contract: ${contractAddress}`); | ||
throw new ContractNotFoundError(contractAddress.toString()); | ||
} | ||
|
||
tree = new ContractTree(contract, this.stateProvider); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import { AztecAddress, Fr, Point } from '@aztec/circuits.js'; | ||
import { Note, randomTxHash } from '@aztec/types'; | ||
|
||
import { DeferredNoteDao } from './deferred_note_dao.js'; | ||
|
||
export const randomDeferredNoteDao = ({ | ||
publicKey = Point.random(), | ||
note = Note.random(), | ||
contractAddress = AztecAddress.random(), | ||
txHash = randomTxHash(), | ||
storageSlot = Fr.random(), | ||
txNullifier = Fr.random(), | ||
newCommitments = [Fr.random(), Fr.random()], | ||
dataStartIndexForTx = Math.floor(Math.random() * 100), | ||
}: Partial<DeferredNoteDao> = {}) => { | ||
return new DeferredNoteDao( | ||
publicKey, | ||
note, | ||
contractAddress, | ||
storageSlot, | ||
txHash, | ||
txNullifier, | ||
newCommitments, | ||
dataStartIndexForTx, | ||
); | ||
}; | ||
|
||
describe('Deferred Note DAO', () => { | ||
it('convert to and from buffer', () => { | ||
const deferredNote = randomDeferredNoteDao(); | ||
const buf = deferredNote.toBuffer(); | ||
expect(DeferredNoteDao.fromBuffer(buf)).toEqual(deferredNote); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
import { AztecAddress, Fr, Point, PublicKey, Vector } from '@aztec/circuits.js'; | ||
import { serializeToBuffer } from '@aztec/circuits.js/utils'; | ||
import { BufferReader, Note, TxHash } from '@aztec/types'; | ||
|
||
/** | ||
* A note that is intended for us, but we cannot decode it yet because the contract is not yet in our database. | ||
* | ||
* So keep the state that we need to decode it later. | ||
*/ | ||
export class DeferredNoteDao { | ||
constructor( | ||
/** The public key associated with this note */ | ||
public publicKey: PublicKey, | ||
/** The note as emitted from the Noir contract. */ | ||
public note: Note, | ||
/** The contract address this note is created in. */ | ||
public contractAddress: AztecAddress, | ||
/** The specific storage location of the note on the contract. */ | ||
public storageSlot: Fr, | ||
/** The hash of the tx the note was created in. */ | ||
public txHash: TxHash, | ||
/** The first nullifier emitted by the transaction */ | ||
public txNullifier: Fr, | ||
/** New commitments in this transaction, one of which belongs to this note */ | ||
public newCommitments: Fr[], | ||
/** The next available leaf index for the note hash tree for this transaction */ | ||
public dataStartIndexForTx: number, | ||
) {} | ||
|
||
toBuffer(): Buffer { | ||
return serializeToBuffer( | ||
this.publicKey.toBuffer(), | ||
this.note.toBuffer(), | ||
this.contractAddress.toBuffer(), | ||
this.storageSlot.toBuffer(), | ||
this.txHash.toBuffer(), | ||
this.txNullifier.toBuffer(), | ||
new Vector(this.newCommitments), | ||
this.dataStartIndexForTx, | ||
); | ||
} | ||
static fromBuffer(buffer: Buffer | BufferReader) { | ||
const reader = BufferReader.asReader(buffer); | ||
return new DeferredNoteDao( | ||
reader.readObject(Point), | ||
reader.readObject(Note), | ||
reader.readObject(AztecAddress), | ||
reader.readObject(Fr), | ||
reader.readObject(TxHash), | ||
reader.readObject(Fr), | ||
reader.readVector(Fr), | ||
reader.readNumber(), | ||
); | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 is great! We should create more custom error classes in the future. They should help us return better error messages through JSON-RPC too.