-
Notifications
You must be signed in to change notification settings - Fork 598
feat(avm): tx hint init #13218
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(avm): tx hint init #13218
Changes from all commits
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 |
|---|---|---|
|
|
@@ -194,8 +194,36 @@ struct EnqueuedCallHint { | |
| MSGPACK_FIELDS(msgSender, contractAddress, calldata, isStaticCall); | ||
| }; | ||
|
|
||
| struct AccumulatedData { | ||
| // TODO: add as needed. | ||
| std::vector<FF> noteHashes; | ||
| std::vector<FF> nullifiers; | ||
|
|
||
| bool operator==(const AccumulatedData& other) const = default; | ||
|
|
||
| MSGPACK_FIELDS(noteHashes, nullifiers); | ||
| }; | ||
|
|
||
| // We are currently using this structure as the input to TX simulation. | ||
| // That's why I'm not calling it TxHint. We can reconsider if the inner types seem to dirty. | ||
| struct Tx { | ||
| AccumulatedData nonRevertibleAccumulatedData; | ||
| AccumulatedData revertibleAccumulatedData; | ||
| std::vector<EnqueuedCallHint> setupEnqueuedCalls; | ||
| std::vector<EnqueuedCallHint> appLogicEnqueuedCalls; | ||
| std::optional<EnqueuedCallHint> teardownEnqueuedCall; | ||
|
|
||
| bool operator==(const Tx& other) const = default; | ||
|
|
||
| MSGPACK_FIELDS(nonRevertibleAccumulatedData, | ||
| revertibleAccumulatedData, | ||
| setupEnqueuedCalls, | ||
| appLogicEnqueuedCalls, | ||
| teardownEnqueuedCall); | ||
| }; | ||
|
Comment on lines
+207
to
+223
Contributor
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. Will this eventually become a complete mirror of the TS
Contributor
Author
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. IDK. It looked like we didn't need everything from the TX object in TS, so I guess this could be a subset? Also, if I wanted to respect the same structure (or if I want to serialize the whole TX) then I have to go in TS and implement (de)serialization/schemas for everything that is in it, even if we don't use it. For the moment I've opted to add what we need, but it can be refactored later if needed. Modifying serialization is really not that difficult now with MP/zod, but it's easier to do it incrementally, I find. Also it lets us look at the structure and see what we already solved and what not (of course not completely true since we don't use note hashes now ;)). |
||
|
|
||
| struct ExecutionHints { | ||
| std::vector<EnqueuedCallHint> enqueuedCalls; | ||
| Tx tx; | ||
| // Contracts. | ||
| std::vector<ContractInstanceHint> contractInstances; | ||
| std::vector<ContractClassHint> contractClasses; | ||
|
|
@@ -213,7 +241,7 @@ struct ExecutionHints { | |
|
|
||
| bool operator==(const ExecutionHints& other) const = default; | ||
|
|
||
| MSGPACK_FIELDS(enqueuedCalls, | ||
| MSGPACK_FIELDS(tx, | ||
| contractInstances, | ||
| contractClasses, | ||
| bytecodeCommitments, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,6 @@ import { computeFeePayerBalanceStorageSlot } from '@aztec/protocol-contracts/fee | |
| import { | ||
| AvmCircuitInputs, | ||
| AvmCircuitPublicInputs, | ||
| AvmEnqueuedCallHint, | ||
| AvmExecutionHints, | ||
| type AvmProvingRequest, | ||
| type RevertCode, | ||
|
|
@@ -72,6 +71,7 @@ export class PublicTxSimulator { | |
| const txHash = await this.computeTxHash(tx); | ||
|
|
||
| this.log.debug(`Simulating ${tx.publicFunctionCalldata.length} public calls for tx ${txHash}`, { txHash }); | ||
|
|
||
| const context = await PublicTxContext.create( | ||
| this.treesDB, | ||
| this.contractsDB, | ||
|
|
@@ -265,18 +265,7 @@ export class PublicTxSimulator { | |
|
|
||
| const allocatedGas = context.getGasLeftAtPhase(phase); | ||
|
|
||
| // The reason we need enqueued hints at all (and cannot just use the public inputs) is | ||
| // because they don't have the actual calldata, just the hash of it. | ||
| // If/when we pass the whole TX to C++, we can remove this class of hints. | ||
| stateManager.traceEnqueuedCall(callRequest.request); | ||
|
Contributor
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. Hmm... i guess we still need to trace the enqueued call because tracing it means "hint that this enqueued call actually happened"? Otherwise, the TX should include everything
Contributor
Author
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. Yep tracing is for PIs I guess? Didn't look too much in detail, maybe it can be removed. |
||
| context.hints.enqueuedCalls.push( | ||
| new AvmEnqueuedCallHint( | ||
| callRequest.request.msgSender, | ||
| contractAddress, | ||
| callRequest.calldata, | ||
| callRequest.request.isStaticCall, | ||
| ), | ||
| ); | ||
|
|
||
| const result = await this.simulateEnqueuedCallInternal( | ||
| context.state.getActiveStateManager(), | ||
|
|
||
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.
TODO meaning "add the remaining side effects as needed?"
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.
And do we need two variations of

AccumulatedDatalike we have in public inputs?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.
I'll respond in the other thread.