feat: global variables in public noir #917
Conversation
| @@ -30,6 +30,10 @@ export class GlobalVariables { | |||
| return new GlobalVariables(...GlobalVariables.getFields(fields)); | |||
| } | |||
|
|
|||
| static empty(): GlobalVariables { | |||
There was a problem hiding this comment.
ik this is already merged, but should we rename GlobalVariables to GlobalContextVariables?
There was a problem hiding this comment.
GlobalBlockContextVariables or maybe GlobalBlockVariables? Context is not super descriptive around what context it is.
There was a problem hiding this comment.
"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.
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.
Will address in a separate pr as it is used a lot of places and don't want to pollute this one.
| @@ -28,8 +28,8 @@ contract Child { | |||
| // 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.
maybe we should make this a seperate test/
There was a problem hiding this comment.
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.
LHerskind
force-pushed
the
lh/globals-noir-public
branch
2 times, most recently
from
41222b2 to
bef50ae
Compare
| new_l2_to_l1_msgs_to_insert[i] = compute_l2_to_l1_hash<NT>(storage_contract_address, | ||
| fr(1), // rollup version id | ||
| private_call_public_inputs.version, |
| @@ -30,6 +30,10 @@ export class GlobalVariables { | |||
| return new GlobalVariables(...GlobalVariables.getFields(fields)); | |||
| } | |||
|
|
|||
| static empty(): GlobalVariables { | |||
There was a problem hiding this comment.
"Global" and "Block" seem opposite? Global would mean always true whereas block variables to me would be things that change per block.
| @@ -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. | |||
| @@ -30,6 +30,10 @@ export class GlobalVariables { | |||
| return new GlobalVariables(...GlobalVariables.getFields(fields)); | |||
| } | |||
|
|
|||
| static empty(): GlobalVariables { | |||
| }); | ||
|
|
||
| it('builds an L2 block using mock simulator', async () => { | ||
| // Assemble a fake transaction | ||
| const txs = await buildMockSimulatorInputs(); | ||
|
|
||
| // Actually build a block! | ||
| const [l2Block, proof] = await builder.buildL2Block(blockNumber, txs, mockL1ToL2Messages); | ||
| const globalVariables = new GlobalVariables(chainId, version, new Fr(blockNumber), Fr.ZERO); |
There was a problem hiding this comment.
This seems able to go within beforeEach
LHerskind
force-pushed
the
lh/globals-noir-public
branch
from
53f1386 to
cd2621b
Compare
Description
Fixes #832.
Note that this is not adding a check for consistency between the global variables used in the public execution and the base rollup, so sequencer can at this point lie his ass off.
The sequencer prepares global variables, but currently the timestamp is not meaningfully constrained and can be antyhing, will be constrained through #830 separately, which also need to modify the value inputted.
Checklist: