-
Notifications
You must be signed in to change notification settings - Fork 265
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: global variables in public noir #917
Changes from all commits
4097f09
529084a
15e36a6
77ea1bf
ad8057e
0f253c1
b917767
7554941
558af75
bc4d4f9
8beb763
19100db
6d4ffa4
cd2621b
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 |
---|---|---|
|
@@ -30,6 +30,10 @@ export class GlobalVariables { | |
return new GlobalVariables(...GlobalVariables.getFields(fields)); | ||
} | ||
|
||
static empty(): GlobalVariables { | ||
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. ik this is already merged, but should we rename GlobalVariables to GlobalContextVariables? 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. "Global" and "Block" seem opposite? Global would mean always true whereas block variables to me would be things that change per block. 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. They are "global" from the POV of noir similarly to how they are in solidity. But some of them change depending on the individual block, so naming might be a little weird. 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 like global block variables 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 address in a separate pr as it is used a lot of places and don't want to pollute this one. |
||
return new GlobalVariables(Fr.zero(), Fr.zero(), Fr.zero(), Fr.zero()); | ||
} | ||
|
||
static fromBuffer(buffer: Buffer | BufferReader): GlobalVariables { | ||
const reader = BufferReader.asReader(buffer); | ||
return new GlobalVariables(reader.readFr(), reader.readFr(), reader.readFr(), reader.readFr()); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
import { createMemDown, getConfigEnvVars } from '@aztec/aztec-node'; | ||
import { | ||
AztecAddress, | ||
GlobalVariables, | ||
KERNEL_NEW_COMMITMENTS_LENGTH, | ||
KERNEL_NEW_L2_TO_L1_MSGS_LENGTH, | ||
KERNEL_NEW_NULLIFIERS_LENGTH, | ||
|
@@ -130,7 +131,7 @@ describe('L1Publisher integration', () => { | |
const vks = getVerificationKeys(); | ||
const simulator = await WasmRollupCircuitSimulator.new(); | ||
const prover = new EmptyRollupProver(); | ||
builder = new SoloBlockBuilder(builderDb, vks, simulator, prover, new Fr(config.chainId), new Fr(config.version)); | ||
builder = new SoloBlockBuilder(builderDb, vks, simulator, prover); | ||
|
||
l2Proof = Buffer.alloc(0); | ||
|
||
|
@@ -182,7 +183,7 @@ describe('L1Publisher integration', () => { | |
}; | ||
|
||
const sendToL2 = async (content: Fr, recipientAddress: AztecAddress) => { | ||
// @todo @LHerskind version hardcoded here | ||
// @todo @LHerskind version hardcoded here (update to bigint or field) | ||
const recipient = new L2Actor(recipientAddress, 1); | ||
// Note: using max deadline | ||
const deadline = 2 ** 32 - 1; | ||
|
@@ -266,7 +267,14 @@ describe('L1Publisher integration', () => { | |
await makeBloatedProcessedTx(128 * i + 96), | ||
await makeBloatedProcessedTx(128 * i + 128), | ||
]; | ||
const [block] = await builder.buildL2Block(1 + i, txs, l1ToL2Messages); | ||
// @todo @LHerskind fix time. | ||
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 stale? 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. |
||
const globalVariables = new GlobalVariables( | ||
new Fr(config.chainId), | ||
new Fr(config.version), | ||
new Fr(1 + i), | ||
Fr.ZERO, | ||
); | ||
const [block] = await builder.buildL2Block(globalVariables, txs, l1ToL2Messages); | ||
|
||
// check that values are in the inbox | ||
for (let j = 0; j < l1ToL2Messages.length; j++) { | ||
|
@@ -351,7 +359,13 @@ describe('L1Publisher integration', () => { | |
await makeEmptyProcessedTx(), | ||
await makeEmptyProcessedTx(), | ||
]; | ||
const [block] = await builder.buildL2Block(1 + i, txs, l1ToL2Messages); | ||
const globalVariables = new GlobalVariables( | ||
new Fr(config.chainId), | ||
new Fr(config.version), | ||
new Fr(1 + i), | ||
Fr.ZERO, | ||
); | ||
const [block] = await builder.buildL2Block(globalVariables, txs, l1ToL2Messages); | ||
|
||
await publisher.processL2Block(block); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,8 +34,8 @@ contract Child { | |
// @param _padding: Nested public functions always get called with MAX_ARGS, since we don't have the ABI available | ||
// during execution time to know how many args are expected, hence the _padding argument. We should | ||
// be able to remove it when we migrate to brillig. | ||
open fn pubValue(_inputs: PublicContextInputs, base_value: Field, _padding: [Field; abi::MAX_ARGS - 1]) -> pub Field { | ||
base_value + 42 | ||
open fn pubValue(inputs: PublicContextInputs, base_value: Field, _padding: [Field; abi::MAX_ARGS - 1]) -> pub Field { | ||
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. maybe we should make this a seperate test/ 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. Were thinking that it was fine as this since we were anyway just returning random values, so might as well return the values from the struct. |
||
base_value + inputs.public_global_variables.chain_id + inputs.public_global_variables.version + inputs.public_global_variables.block_number + inputs.public_global_variables.timestamp | ||
} | ||
|
||
// Increments `current_value` by `new_value` and returns `new_value` + 1. | ||
|
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.
nice!