Skip to content
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: slot duration flexibility #8122

Merged
merged 3 commits into from
Aug 27, 2024
Merged

feat: slot duration flexibility #8122

merged 3 commits into from
Aug 27, 2024

Conversation

LHerskind
Copy link
Contributor

@LHerskind LHerskind commented Aug 21, 2024

This PR add support for using Aztec slot durations that are not just 1 ethereum slot.

To do so it adds more logic to the sequencer such that they can figure out if they are really the sequencer or not.
This is mainly in the canProposeBlock and shouldProposeBlock functions which respectively assert that the proposer can actually propose, and whether he should. The logic in those functions are partly what we had sprinkled over the code before, but also adding some additional checks to match what is in the rollup contract and Leonidas.

Since there are now additional restrictions to block production related to WHEN the block is to land on L1, I have added a commitTimeJump function into the l1-publisherwhich will jump to the next slot after a block have been published. This functionality toggled with a TIME_TRAVELER boolean flag.

Note that we will jump INTO the next slot, since simulations in viem and anvil are limited to run on block values in the bast, meaning that we cannot nicely just simulate as if it was included in the NEXT block which is what we ideally want. See issue #8110 for more information.

Since it caused some issues that there was no actual genesis state (just 0), I have inserted a genesis state equal to what we compute as the lastArchive for the very first block, fixing #4148.

To not mess too much with DEVNET, the extra logic related to the exact timing of when L2 blocks should make it onto L1 can be "toggled" with the IS_DEV_NET flag.

Namely, if IS_DEV_NET is toggled, we can publish outside of the "current" slot, as long as the slots are in order. With the changes in this pr, we should be able to run DEVNET without automine, I have tried a minor test but that seemed to work fine when we have AZTEC_SLOT_DURATION = 36 and internal mining :)

Points of interest:

  • sequencer.ts
    • canProposeBlock
    • shouldProposeBlock
  • l1-publisher.ts
    • Adding simulate since viem write does not provide meaningful error messages, but simulate does.
    • The commitTimeJump won't happen if the AZTEC_SLOT_DURATION = ETHEREUM_SLOT_DURATION or TIME_TRAVELER = false
  • sequencer.test.ts
    • Some of the test naming a odd. For example the test builds a block that contains zero real transactions once flushed sounds to me like you expect to have an empty block, after the block flushed, e.g., once flushed make it sound like it already happened
  • l1-publisher.test.ts
    • does not publish if last archive root is different to expected is deleted, as that job falls on the sequencer. The sequencer should define whether or not it will send a tx, and publisher deals more with publishing tasks.
    • Adding a simulate that is also mocked to account for simulate in the publisher.
  • e2e_p2p_network.test.ts
    • There is a flag for using a local anvil chain that you are running in another terminal. This is useful for running internal mining etc.

This PR will be run with AZTEC_SLOT_DURATION = 12 and IS_DEV_NET = true. Note that when using values different from those, there are still some hiccups and kinks, but it should be addressed in a separate PR to not make this explode in size.

Copy link
Contributor Author

LHerskind commented Aug 21, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @LHerskind and the rest of your teammates on Graphite Graphite

@AztecBot
Copy link
Collaborator

AztecBot commented Aug 21, 2024

Benchmark results

Metrics with a significant change:

  • proof_construction_time_sha256_100_ms (16): 7,409 (+36%)
  • proof_construction_time_sha256_100_ms (32): 7,211 (+33%)
Detailed results

All benchmarks are run on txs on the Benchmarking contract on the repository. Each tx consists of a batch call to create_note and increment_balance, which guarantees that each tx has a private call, a nested private call, a public call, and a nested public call, as well as an emitted private note, an unencrypted log, and public storage read and write.

This benchmark source data is available in JSON format on S3 here.

Proof generation

Each column represents the number of threads used in proof generation.

Metric 1 threads 4 threads 16 threads 32 threads 64 threads
proof_construction_time_sha256_ms 5,876 (+2%) 1,586 (+2%) 743 (+5%) 800 (+5%) 785 (+1%)
proof_construction_time_sha256_30_ms 12,343 (+8%) 3,425 (+11%) 1,547 (+13%) 1,550 (+8%) 1,607 (+10%)
proof_construction_time_sha256_100_ms 45,453 (+3%) 13,241 (+12%) ⚠️ 7,409 (+36%) ⚠️ 7,211 (+33%) 6,323 (+10%)
proof_construction_time_poseidon_hash_ms 79.0 (+1%) 34.0 34.0 58.0 86.0 (-2%)
proof_construction_time_poseidon_hash_30_ms 1,540 (+1%) 424 (+1%) 203 229 (+2%) 273 (+2%)
proof_construction_time_poseidon_hash_100_ms 5,778 (+2%) 1,542 (+2%) 694 (+3%) 738 (-1%) 770 (+4%)

L2 block published to L1

Each column represents the number of txs on an L2 block published to L1.

Metric 4 txs 8 txs 16 txs
l1_rollup_calldata_size_in_bytes 4,356 7,876 14,884
l1_rollup_calldata_gas 50,244 92,966 178,240
l1_rollup_execution_gas 842,904 1,576,522 3,361,852
l2_block_processing_time_in_ms 251 (-3%) 460 (+5%) 819 (-3%)
l2_block_building_time_in_ms 11,314 22,169 44,125 (-1%)
l2_block_rollup_simulation_time_in_ms 11,313 22,168 44,125 (-1%)
l2_block_public_tx_process_time_in_ms 9,582 (-1%) 20,347 42,294 (-1%)

L2 chain processing

Each column represents the number of blocks on the L2 chain where each block has 8 txs.

Metric 3 blocks 5 blocks
node_history_sync_time_in_ms 3,012 (+3%) 3,931 (+3%)
node_database_size_in_bytes 12,615,760 16,732,240
pxe_database_size_in_bytes 16,254 26,813

Circuits stats

Stats on running time and I/O sizes collected for every kernel circuit run across all benchmarks.

Circuit simulation_time_in_ms witness_generation_time_in_ms input_size_in_bytes output_size_in_bytes proving_time_in_ms
private-kernel-init 90.8 (-1%) 396 (-1%) 21,735 44,860 N/A
private-kernel-inner 177 (+1%) 706 (-1%) 72,544 45,007 N/A
private-kernel-reset-tiny 312 717 65,593 44,846 N/A
private-kernel-tail 167 (+1%) 139 50,644 52,257 N/A
base-parity 5.59 N/A 160 96.0 N/A
root-parity 35.3 N/A 73,948 96.0 N/A
base-rollup 3,429 N/A 189,136 664 N/A
block-root-rollup 41.4 N/A 58,205 2,448 N/A
public-kernel-setup 85.2 N/A 105,085 71,222 N/A
public-kernel-app-logic 96.7 N/A 104,911 71,222 N/A
public-kernel-tail 859 N/A 390,582 16,414 N/A
private-kernel-reset-small 314 (+1%) N/A 66,341 45,629 N/A
private-kernel-tail-to-public 671 614 455,400 1,825 N/A
public-kernel-teardown 84.5 (+1%) N/A 105,349 71,222 N/A
merge-rollup 20.3 N/A 38,174 664 N/A
undefined N/A N/A N/A N/A 77,332 (-1%)

Stats on running time collected for app circuits

Function input_size_in_bytes output_size_in_bytes witness_generation_time_in_ms
ContractClassRegisterer:register 1,344 11,731 343 (+1%)
ContractInstanceDeployer:deploy 1,408 11,731 18.4
MultiCallEntrypoint:entrypoint 1,920 11,731 406 (+1%)
FeeJuice:deploy 1,376 11,731 394 (+1%)
SchnorrAccount:constructor 1,312 11,731 73.6 (+1%)
SchnorrAccount:entrypoint 2,304 11,731 396
Token:privately_mint_private_note 1,280 11,731 104
FPC:fee_entrypoint_public 1,344 11,731 27.9 (+7%)
Token:transfer 1,312 11,731 235 (+3%)
Benchmarking:create_note 1,344 11,731 85.2 (-2%)
SchnorrAccount:verify_private_authwit 1,280 11,731 27.7 (+1%)
Token:unshield 1,376 11,731 527 (+1%)
FPC:fee_entrypoint_private 1,376 11,731 703 (-4%)

AVM Simulation

Time to simulate various public functions in the AVM.

Function time_ms bytecode_size_in_bytes
FeeJuice:_increase_public_balance 57.1 (+4%) 7,101
FeeJuice:set_portal 11.1 (+15%) 2,128
Token:constructor 81.4 (+3%) 25,285
FPC:constructor 53.2 (+1%) 17,853
FeeJuice:mint_public 39.9 (+2%) 5,415
Token:mint_public 44.7 (+1%) 10,101
Token:assert_minter_and_mint 67.8 (+4%) 6,844
AuthRegistry:set_authorized 48.0 3,969
FPC:prepare_fee 257 (+9%) 6,747
Token:transfer_public 27.6 (+1%) 38,126
FPC:pay_refund 51.0 (-4%) 9,398
Benchmarking:increment_balance 1,194 6,179
Token:_increase_public_balance 41.5 (+2%) 7,705
FPC:pay_refund_with_shielded_rebate 63.1 (+4%) 9,881

Public DB Access

Time to access various public DBs.

Function time_ms
get-nullifier-index 0.160 (+3%)

Tree insertion stats

The duration to insert a fixed batch of leaves into each tree type.

Metric 1 leaves 16 leaves 64 leaves 128 leaves 256 leaves 512 leaves 1024 leaves
batch_insert_into_append_only_tree_16_depth_ms 2.20 (+1%) 3.86 N/A N/A N/A N/A N/A
batch_insert_into_append_only_tree_16_depth_hash_count 16.8 31.7 N/A N/A N/A N/A N/A
batch_insert_into_append_only_tree_16_depth_hash_ms 0.115 (+2%) 0.109 N/A N/A N/A N/A N/A
batch_insert_into_append_only_tree_32_depth_ms N/A N/A 11.3 (+1%) 17.7 (-1%) 30.6 (+1%) 59.9 (+1%) 113 (-1%)
batch_insert_into_append_only_tree_32_depth_hash_count N/A N/A 95.9 159 287 543 1,055
batch_insert_into_append_only_tree_32_depth_hash_ms N/A N/A 0.108 (+1%) 0.103 (-1%) 0.0995 (+1%) 0.103 (+1%) 0.101 (-1%)
batch_insert_into_indexed_tree_20_depth_ms N/A N/A 14.2 25.9 (+2%) 43.6 (+1%) 86.5 (+6%) 165 (+2%)
batch_insert_into_indexed_tree_20_depth_hash_count N/A N/A 109 207 355 691 1,363
batch_insert_into_indexed_tree_20_depth_hash_ms N/A N/A 0.107 (-1%) 0.104 (+2%) 0.106 (+1%) 0.106 (+5%) 0.104 (+1%)
batch_insert_into_indexed_tree_40_depth_ms N/A N/A 16.7 N/A N/A N/A N/A
batch_insert_into_indexed_tree_40_depth_hash_count N/A N/A 132 N/A N/A N/A N/A
batch_insert_into_indexed_tree_40_depth_hash_ms N/A N/A 0.107 (+1%) N/A N/A N/A N/A

Miscellaneous

Transaction sizes based on how many contract classes are registered in the tx.

Metric 0 registered classes 1 registered classes
tx_size_in_bytes 64,779 668,997

Transaction size based on fee payment method

| Metric | |
| - | |

@LHerskind
Copy link
Contributor Author

@spalladino I recall you mentioned at some point to let you know if new flags etc 👀 I have a new flag here, it is effective if we are using AZTEC_SLOT_DURATION != ETHEREUM_SLOT_DURATION but thought you might wanna know anyway.

@@ -132,8 +132,10 @@ global INITIALIZATION_SLOT_SEPARATOR: Field = 1000_000_000;
global INITIAL_L2_BLOCK_NUM: Field = 1;
global BLOB_SIZE_IN_BYTES: Field = 31 * 4096;
global ETHEREUM_SLOT_DURATION: u32 = 12;
global AZTEC_SLOT_DURATION: u32 = ETHEREUM_SLOT_DURATION * 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the need for the * 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Want the AZTEC_SLOT_DURATION to be a multiple of a ETHEREUM_SLOT_DURATION so just makes it very easy to change it to 3 to have the different duration 🤷 Is not really needed when the value is 1, but just kinda nice to showcase multiple I think.

Copy link
Member

@Maddiaa0 Maddiaa0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall great work, I'm not the biggest fan of the wrap logic living within the node itself, but it seems to be a necessary evil

@@ -88,7 +88,11 @@ contract Rollup is Leonidas, IRollup, ITestRollup {
VERSION = 1;

// Genesis block
blocks[0] = BlockLog({archive: bytes32(0), slotNumber: 0, isProven: true});
blocks[0] = BlockLog({
archive: bytes32(0x1200a06aae1368abe36530b585bd7a4d2ba4de5037b82076412691a187d7621e),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this calcualted from ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the lastArchive that all tests will spit out for block 1, e.g., what initial state of the trees.

@@ -77,6 +77,9 @@ const config = getConfigEnvVars();

const numberOfConsecutiveBlocks = 2;

// The initial archive is what we have in the genesis block in `Rollup.sol`.
const INITIAL_ARCHIVE = Fr.fromString('0x1200a06aae1368abe36530b585bd7a4d2ba4de5037b82076412691a187d7621e').toBuffer();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would this make sense to go in the constants if it will be hardcoded as such

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ye probably a fine idea.

@@ -169,6 +172,7 @@ describe('L1Publisher integration', () => {
publisherPrivateKey: sequencerPK,
l1PublishRetryIntervalMS: 100,
l1ChainId: 31337,
timeTraveler: true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we add an issue to remove this at some point

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #8153.

@@ -270,14 +301,55 @@ export class L1Publisher {
return false;
}

async commitTimeJump(slot: bigint) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it enough just to have test utilities to calaulcate / do this, rather than embed this inside the publisher itself?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess it needs to go in here to make the other tests pass within IS_DEV_NET

Copy link
Contributor Author

@LHerskind LHerskind Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason it was in the publisher itself was that it was something we needed to do when a block was published (or for the simulation issue), so keeping it there gave better control.

Alternatively could be running in the background in the tests to progress blocks if nothing have happened for 12 seconds essentially making a fallback to the interval mining, which would be slower unless it also look into jumping the required time for slots etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried it out. Works fine for handling the "slot already filled", but non-obvious how to check for the simulation failure without rebuilding a lot of the sequencer, so just had a "progress block at least every 12 seconds". It will kinda work, but it is really slow if it start running into the "have to wait" case.

const result = await publisher.processL2Block(l2Block);

expect(result).toEqual(false);
expect(rollupContractWrite.process).toHaveBeenCalledTimes(1);
});

it('does not retry if simulating a publish and process tx fails', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

client: walletClient,
});

// @note We make a time jump PAST the very first slot to not have to deal with the edge case of the first slot
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain why the first slot is such an edge case, not necessarily in the code, but to me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The genesis block is at slot 0, so the slot is already occupied.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That feels like a worthy comment in the code

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update: necessarily in the code

@@ -47,7 +48,13 @@ describe('e2e_p2p_network', () => {
let deployL1ContractsValues: DeployL1Contracts;

beforeEach(async () => {
({ teardown, config, logger, deployL1ContractsValues } = await setup(0));
// If we want to test with interval mining, we can use the local host and start `anvil --block-time 12`
const useLocalHost = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could have the interval mining settings added to the createAnvilOpts instead? So it is not as siloed to this test

Copy link
Contributor Author

@LHerskind LHerskind Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was not always for interval, sometimes also just used it to get proper errors back. For cases like the e2e_token_contract I think it might cause quite a bit of pain if they are all using the same local anvil for example (unless interval mining ye). The case is only really interesting for testing the sequencing which is currently only happening in this test.

@trackSpan('Sequencer.buildBlockAndPublish', (_validTxs, newGlobalVariables, _historicalHeader) => ({
[Attributes.BLOCK_NUMBER]: newGlobalVariables.blockNumber.toNumber(),
}))
async canProposeBlock(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice function

return true;
}

async assertCanStillProposeBlock(): Promise<void> {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bollocks.

@@ -128,7 +125,7 @@ contract SpartaTest is DecoderBase {
return;
}

_testBlock("mixed_block_1", false, 0, false); // We run a block before the epoch with validators
_testBlock("mixed_block_1", false, 3, false); // We run a block before the epoch with validators
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead comment

@@ -463,13 +469,13 @@ describe('L1Publisher integration', () => {
nextL1ToL2Messages = [];

// @todo @LHerskind need to make sure that time have progressed to the next slot!
await setTimeToNextSlot();
await progressTimeBySlot();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just to help the next test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its running a loop and publishing to the contract so if the contract it unhappy it would make the test fail. The underlying chain just have to be progressed, when the block time are equal for the two there are no issues, but it gets weird when they differ without this one.

client: walletClient,
});

// @note We make a time jump PAST the very first slot to not have to deal with the edge case of the first slot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That feels like a worthy comment in the code

const submitter = await this.getProposerAtNextEthBlock();
const sender = await this.getSenderAddress();
return submitter.isZero() || submitter.equals(sender);
// @note Assumes that all ethereum slots have blocks
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we create an issue to test the case where L1 has empty slots?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if anvil allow you to do this. The issue that might be encountered if this is the very last check before publishing could be that you are publishing a tx that will end up reverting. Would happen if the last slot you could include your tx in is also the one that is skipped.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think we'd just need to kill anvil for a period of time and then restore it.

}

// If the aztec slot duration is same length as the ethereum slot duration, we don't need to do anything
if ((ETHEREUM_SLOT_DURATION as number) === (AZTEC_SLOT_DURATION as number)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doesn't this help us when they are the same?
For example, suppose L1 is at t=13 seconds after genesis.
We just published the block in slot 1.
Would we not want to jump L1 to t=24?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, the next block will already be at 24, you don't need to jump to it. Also, I believe you mean t=13 for the first?

return submitter.isZero() || submitter.equals(sender);
// @note Assumes that all ethereum slots have blocks
// Using next Ethereum block so we do NOT need to wait for it being mined before seeing the effect
public async getMetadataForSlotAtNextEthBlock(): Promise<[EthAddress, bigint, bigint, Buffer]> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally dislike returning tuples like this especially when there are repeated types (bigint, bigint).

For example, it took me a minute to understand you're returning the pendingBlockNumber.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can change it to a struct.

throw new Error(msg);
}

const currentBlockNumber = await this.l2BlockSource.getBlockNumber();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How could this happen if we have ensured that L1 is expecting this block number?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const msg = 'New block was emitted while building block';

If anyone else published a block.


if (IS_DEV_NET) {
// Compute time elapsed since the previous block
const lastBlockTime = historicalHeader?.globalVariables.timestamp.toNumber() || 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like duplication of skipMinTxsPerBlockCheck

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Different variables. minSecondsBetweenBlocks != maxSecondsBetweenBlocks.

let attestations: BlockAttestation[] = [];
this.log.info(`Waiting for ${numberOfRequiredAttestations} attestations for slot: ${slot}`);

let attestations: BlockAttestation[] = [await this.attestToProposal(proposal)];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always attest to my own proposal?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The smart contract does not filter the proposer from the expected signatures, this could probably be removed by doing the filtering

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

He is a member of the committee, so he can attest 🤷 Pretty sure the same is the case for other systems as well. If you are running small committees it is also a requirement to really hit the quorum needed.

@spalladino
Copy link
Collaborator

@LHerskind thanks for the heads up. Not sure I followed though: the new flag is the TIME_TRAVELER one, or the AZTEC_SLOT_DURATION? What should we set for devnet/provernet, or if IS_DEVNET == 1 then we can skip them?

@LHerskind
Copy link
Contributor Author

@LHerskind thanks for the heads up. Not sure I followed though: the new flag is the TIME_TRAVELER one, or the AZTEC_SLOT_DURATION? What should we set for devnet/provernet, or if IS_DEVNET == 1 then we can skip them?

In here it is TIME_TRAVELER as flag, the other values are constants in the constants.nr.

Currently, we are taking a look to see if we can rip it out, such that you don't have to deal with it at all.

@LHerskind
Copy link
Contributor Author

@LHerskind thanks for the heads up. Not sure I followed though: the new flag is the TIME_TRAVELER one, or the AZTEC_SLOT_DURATION? What should we set for devnet/provernet, or if IS_DEVNET == 1 then we can skip them?

Following #8193 I'm not really sure around the setup that you are using for the devnet/provernet, since I would expect you to be running it with the same block times as Eth but the issues encountered there seems to be mainly related to timing of the underlying not matching what was expected.

If the AZTEC_SLOT_DURATION = ETHEREUM_SLOT_DURATION = ACTUAL_ETHEREUM_SLOT_DURATION_IN_DEVNET then TIME_TRAVELER = false is fine with IS_DEVNET = true.

@LHerskind LHerskind merged commit 708e4e5 into master Aug 27, 2024
98 checks passed
@LHerskind LHerskind deleted the lh/7850-slot-duration branch August 27, 2024 14:06
TomAFrench pushed a commit that referenced this pull request Aug 29, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.51.1</summary>

##
[0.51.1](aztec-package-v0.51.0...aztec-package-v0.51.1)
(2024-08-29)


### Features

* Add status check to prover agent
([#8248](#8248))
([7b3006a](7b3006a))
* Faster L1 deployment
([#8234](#8234))
([51d6699](51d6699))
* Spartan token transfer
([#8163](#8163))
([38f0157](38f0157))
</details>

<details><summary>barretenberg.js: 0.51.1</summary>

##
[0.51.1](barretenberg.js-v0.51.0...barretenberg.js-v0.51.1)
(2024-08-29)


### Miscellaneous

* **barretenberg.js:** Synchronize aztec-packages versions
</details>

<details><summary>aztec-packages: 0.51.1</summary>

##
[0.51.1](aztec-packages-v0.51.0...aztec-packages-v0.51.1)
(2024-08-29)


### Features

* Add CLI command for gathering proving metrics
([#8221](#8221))
([5929a42](5929a42))
* Add status check to prover agent
([#8248](#8248))
([7b3006a](7b3006a))
* **avm:** 1-slot sload/sstore (nr, ts)
([#8264](#8264))
([bdd9b06](bdd9b06))
* **avm:** Range check gadget
([#7967](#7967))
([0dd954e](0dd954e))
* **docs:** Add partial notes doc
([#8192](#8192))
([4299bbd](4299bbd))
* Faster L1 deployment
([#8234](#8234))
([51d6699](51d6699))
* Initial validator set
([#8133](#8133))
([6d31ad2](6d31ad2))
* L1-publisher cleanup
([#8148](#8148))
([6ae2535](6ae2535))
* Proof surgery class
([#8236](#8236))
([10d7edd](10d7edd))
* Request specific transactions through the p2p layer
([#8185](#8185))
([54e1cc7](54e1cc7))
* Slot duration flexibility
([#8122](#8122))
([708e4e5](708e4e5))
* Spartan token transfer
([#8163](#8163))
([38f0157](38f0157))


### Bug Fixes

* Attempt to fix nightly test
([#8222](#8222))
([477eec5](477eec5))
* **avm-simulator:** Await avm bytecode check
([#8268](#8268))
([4410eb3](4410eb3))
* **bb-prover:** Create structure for AVM vk
([#8233](#8233))
([55b6ba2](55b6ba2))
* **bb:** Mac build
([#8255](#8255))
([ac54f5c](ac54f5c))
* **ci:** Spot-runner-action was not built
([#8274](#8274))
([c1509c1](c1509c1))
* **ci:** Try fix brotli edge-case
([#8256](#8256))
([e03ea0b](e03ea0b))
* Docker containers healthchecks
([#8228](#8228))
([19edbbb](19edbbb))
* **docs:** Update entrypoint details on accounts page
([#8184](#8184))
([8453ec7](8453ec7))
* Export brillig names in contract functions
([#8212](#8212))
([4745741](4745741))
* Fixes for the nightly test run against Sepolia
([#8229](#8229))
([cfc65c6](cfc65c6))
* Handle constant output for sha256
([#8251](#8251))
([0653ba5](0653ba5))
* Log public vm errors as warn in prover-agent
([#8247](#8247))
([9f4ea9f](9f4ea9f))
* Remove devnet ARM builds for now
([#8202](#8202))
([81ef715](81ef715))
* Remove fundFpc step from bootstrap
([#8245](#8245))
([a742531](a742531))
* Ts codegen
([#8267](#8267))
([cb58800](cb58800))


### Miscellaneous

* Add check to just release images to devnet-deploys
([#8242](#8242))
([aa6791d](aa6791d))
* Add partial note support for value note
([#8141](#8141))
([daa57cc](daa57cc))
* Always run `build-check` step in `publish-bb.yml`
([#8240](#8240))
([5e9749f](5e9749f))
* **avm:** Replace range and cmp with gadgets
([#8164](#8164))
([cc12558](cc12558))
* Basic network matrix
([#8257](#8257))
([2a76b1a](2a76b1a)),
closes
[#8001](#8001)
* **bb:** Use std::span in pippenger for scalars
([#8269](#8269))
([2323cd5](2323cd5))
* Configure interval mining for anvil
([#8211](#8211))
([eba57b4](eba57b4))
* Create external-ci-approved.yml
([#8235](#8235))
([24b059b](24b059b))
* Disallow prune in devnet + add onlyOwners
([#8134](#8134))
([c736f96](c736f96))
* Fix various warnings in noir code
([#8258](#8258))
([1c6b478](1c6b478))
* Less noisy AVM failures in proving
([#8227](#8227))
([03bcd62](03bcd62))
* Open an issue if publishing bb fails
([#8223](#8223))
([2d7a775](2d7a775))
* Reinstate l1-contracts package
([#8250](#8250))
([263a912](263a912))
* Remove unused generic parameters
([#8249](#8249))
([00ed045](00ed045))
* Replace relative paths to noir-protocol-circuits
([1783c80](1783c80))
* Replace relative paths to noir-protocol-circuits
([ffe1f35](ffe1f35))
* Report prover metrics
([#8155](#8155))
([dc7bcdf](dc7bcdf)),
closes
[#7675](#7675)
* Rework balances map
([#8127](#8127))
([1cac3dd](1cac3dd)),
closes
[#8104](#8104)
* Run CI after merges to provernet
([#8244](#8244))
([97e5e25](97e5e25))


### Documentation

* Minor fixes
([#8273](#8273))
([2b8af9e](2b8af9e))
</details>

<details><summary>barretenberg: 0.51.1</summary>

##
[0.51.1](barretenberg-v0.51.0...barretenberg-v0.51.1)
(2024-08-29)


### Features

* **avm:** 1-slot sload/sstore (nr, ts)
([#8264](#8264))
([bdd9b06](bdd9b06))
* **avm:** Range check gadget
([#7967](#7967))
([0dd954e](0dd954e))
* Proof surgery class
([#8236](#8236))
([10d7edd](10d7edd))


### Bug Fixes

* **bb-prover:** Create structure for AVM vk
([#8233](#8233))
([55b6ba2](55b6ba2))
* **bb:** Mac build
([#8255](#8255))
([ac54f5c](ac54f5c))
* Handle constant output for sha256
([#8251](#8251))
([0653ba5](0653ba5))


### Miscellaneous

* **avm:** Replace range and cmp with gadgets
([#8164](#8164))
([cc12558](cc12558))
* **bb:** Use std::span in pippenger for scalars
([#8269](#8269))
([2323cd5](2323cd5))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
AztecBot added a commit to AztecProtocol/barretenberg that referenced this pull request Aug 30, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.51.1</summary>

##
[0.51.1](AztecProtocol/aztec-packages@aztec-package-v0.51.0...aztec-package-v0.51.1)
(2024-08-29)


### Features

* Add status check to prover agent
([#8248](AztecProtocol/aztec-packages#8248))
([7b3006a](AztecProtocol/aztec-packages@7b3006a))
* Faster L1 deployment
([#8234](AztecProtocol/aztec-packages#8234))
([51d6699](AztecProtocol/aztec-packages@51d6699))
* Spartan token transfer
([#8163](AztecProtocol/aztec-packages#8163))
([38f0157](AztecProtocol/aztec-packages@38f0157))
</details>

<details><summary>barretenberg.js: 0.51.1</summary>

##
[0.51.1](AztecProtocol/aztec-packages@barretenberg.js-v0.51.0...barretenberg.js-v0.51.1)
(2024-08-29)


### Miscellaneous

* **barretenberg.js:** Synchronize aztec-packages versions
</details>

<details><summary>aztec-packages: 0.51.1</summary>

##
[0.51.1](AztecProtocol/aztec-packages@aztec-packages-v0.51.0...aztec-packages-v0.51.1)
(2024-08-29)


### Features

* Add CLI command for gathering proving metrics
([#8221](AztecProtocol/aztec-packages#8221))
([5929a42](AztecProtocol/aztec-packages@5929a42))
* Add status check to prover agent
([#8248](AztecProtocol/aztec-packages#8248))
([7b3006a](AztecProtocol/aztec-packages@7b3006a))
* **avm:** 1-slot sload/sstore (nr, ts)
([#8264](AztecProtocol/aztec-packages#8264))
([bdd9b06](AztecProtocol/aztec-packages@bdd9b06))
* **avm:** Range check gadget
([#7967](AztecProtocol/aztec-packages#7967))
([0dd954e](AztecProtocol/aztec-packages@0dd954e))
* **docs:** Add partial notes doc
([#8192](AztecProtocol/aztec-packages#8192))
([4299bbd](AztecProtocol/aztec-packages@4299bbd))
* Faster L1 deployment
([#8234](AztecProtocol/aztec-packages#8234))
([51d6699](AztecProtocol/aztec-packages@51d6699))
* Initial validator set
([#8133](AztecProtocol/aztec-packages#8133))
([6d31ad2](AztecProtocol/aztec-packages@6d31ad2))
* L1-publisher cleanup
([#8148](AztecProtocol/aztec-packages#8148))
([6ae2535](AztecProtocol/aztec-packages@6ae2535))
* Proof surgery class
([#8236](AztecProtocol/aztec-packages#8236))
([10d7edd](AztecProtocol/aztec-packages@10d7edd))
* Request specific transactions through the p2p layer
([#8185](AztecProtocol/aztec-packages#8185))
([54e1cc7](AztecProtocol/aztec-packages@54e1cc7))
* Slot duration flexibility
([#8122](AztecProtocol/aztec-packages#8122))
([708e4e5](AztecProtocol/aztec-packages@708e4e5))
* Spartan token transfer
([#8163](AztecProtocol/aztec-packages#8163))
([38f0157](AztecProtocol/aztec-packages@38f0157))


### Bug Fixes

* Attempt to fix nightly test
([#8222](AztecProtocol/aztec-packages#8222))
([477eec5](AztecProtocol/aztec-packages@477eec5))
* **avm-simulator:** Await avm bytecode check
([#8268](AztecProtocol/aztec-packages#8268))
([4410eb3](AztecProtocol/aztec-packages@4410eb3))
* **bb-prover:** Create structure for AVM vk
([#8233](AztecProtocol/aztec-packages#8233))
([55b6ba2](AztecProtocol/aztec-packages@55b6ba2))
* **bb:** Mac build
([#8255](AztecProtocol/aztec-packages#8255))
([ac54f5c](AztecProtocol/aztec-packages@ac54f5c))
* **ci:** Spot-runner-action was not built
([#8274](AztecProtocol/aztec-packages#8274))
([c1509c1](AztecProtocol/aztec-packages@c1509c1))
* **ci:** Try fix brotli edge-case
([#8256](AztecProtocol/aztec-packages#8256))
([e03ea0b](AztecProtocol/aztec-packages@e03ea0b))
* Docker containers healthchecks
([#8228](AztecProtocol/aztec-packages#8228))
([19edbbb](AztecProtocol/aztec-packages@19edbbb))
* **docs:** Update entrypoint details on accounts page
([#8184](AztecProtocol/aztec-packages#8184))
([8453ec7](AztecProtocol/aztec-packages@8453ec7))
* Export brillig names in contract functions
([#8212](AztecProtocol/aztec-packages#8212))
([4745741](AztecProtocol/aztec-packages@4745741))
* Fixes for the nightly test run against Sepolia
([#8229](AztecProtocol/aztec-packages#8229))
([cfc65c6](AztecProtocol/aztec-packages@cfc65c6))
* Handle constant output for sha256
([#8251](AztecProtocol/aztec-packages#8251))
([0653ba5](AztecProtocol/aztec-packages@0653ba5))
* Log public vm errors as warn in prover-agent
([#8247](AztecProtocol/aztec-packages#8247))
([9f4ea9f](AztecProtocol/aztec-packages@9f4ea9f))
* Remove devnet ARM builds for now
([#8202](AztecProtocol/aztec-packages#8202))
([81ef715](AztecProtocol/aztec-packages@81ef715))
* Remove fundFpc step from bootstrap
([#8245](AztecProtocol/aztec-packages#8245))
([a742531](AztecProtocol/aztec-packages@a742531))
* Ts codegen
([#8267](AztecProtocol/aztec-packages#8267))
([cb58800](AztecProtocol/aztec-packages@cb58800))


### Miscellaneous

* Add check to just release images to devnet-deploys
([#8242](AztecProtocol/aztec-packages#8242))
([aa6791d](AztecProtocol/aztec-packages@aa6791d))
* Add partial note support for value note
([#8141](AztecProtocol/aztec-packages#8141))
([daa57cc](AztecProtocol/aztec-packages@daa57cc))
* Always run `build-check` step in `publish-bb.yml`
([#8240](AztecProtocol/aztec-packages#8240))
([5e9749f](AztecProtocol/aztec-packages@5e9749f))
* **avm:** Replace range and cmp with gadgets
([#8164](AztecProtocol/aztec-packages#8164))
([cc12558](AztecProtocol/aztec-packages@cc12558))
* Basic network matrix
([#8257](AztecProtocol/aztec-packages#8257))
([2a76b1a](AztecProtocol/aztec-packages@2a76b1a)),
closes
[#8001](AztecProtocol/aztec-packages#8001)
* **bb:** Use std::span in pippenger for scalars
([#8269](AztecProtocol/aztec-packages#8269))
([2323cd5](AztecProtocol/aztec-packages@2323cd5))
* Configure interval mining for anvil
([#8211](AztecProtocol/aztec-packages#8211))
([eba57b4](AztecProtocol/aztec-packages@eba57b4))
* Create external-ci-approved.yml
([#8235](AztecProtocol/aztec-packages#8235))
([24b059b](AztecProtocol/aztec-packages@24b059b))
* Disallow prune in devnet + add onlyOwners
([#8134](AztecProtocol/aztec-packages#8134))
([c736f96](AztecProtocol/aztec-packages@c736f96))
* Fix various warnings in noir code
([#8258](AztecProtocol/aztec-packages#8258))
([1c6b478](AztecProtocol/aztec-packages@1c6b478))
* Less noisy AVM failures in proving
([#8227](AztecProtocol/aztec-packages#8227))
([03bcd62](AztecProtocol/aztec-packages@03bcd62))
* Open an issue if publishing bb fails
([#8223](AztecProtocol/aztec-packages#8223))
([2d7a775](AztecProtocol/aztec-packages@2d7a775))
* Reinstate l1-contracts package
([#8250](AztecProtocol/aztec-packages#8250))
([263a912](AztecProtocol/aztec-packages@263a912))
* Remove unused generic parameters
([#8249](AztecProtocol/aztec-packages#8249))
([00ed045](AztecProtocol/aztec-packages@00ed045))
* Replace relative paths to noir-protocol-circuits
([1783c80](AztecProtocol/aztec-packages@1783c80))
* Replace relative paths to noir-protocol-circuits
([ffe1f35](AztecProtocol/aztec-packages@ffe1f35))
* Report prover metrics
([#8155](AztecProtocol/aztec-packages#8155))
([dc7bcdf](AztecProtocol/aztec-packages@dc7bcdf)),
closes
[#7675](AztecProtocol/aztec-packages#7675)
* Rework balances map
([#8127](AztecProtocol/aztec-packages#8127))
([1cac3dd](AztecProtocol/aztec-packages@1cac3dd)),
closes
[#8104](AztecProtocol/aztec-packages#8104)
* Run CI after merges to provernet
([#8244](AztecProtocol/aztec-packages#8244))
([97e5e25](AztecProtocol/aztec-packages@97e5e25))


### Documentation

* Minor fixes
([#8273](AztecProtocol/aztec-packages#8273))
([2b8af9e](AztecProtocol/aztec-packages@2b8af9e))
</details>

<details><summary>barretenberg: 0.51.1</summary>

##
[0.51.1](AztecProtocol/aztec-packages@barretenberg-v0.51.0...barretenberg-v0.51.1)
(2024-08-29)


### Features

* **avm:** 1-slot sload/sstore (nr, ts)
([#8264](AztecProtocol/aztec-packages#8264))
([bdd9b06](AztecProtocol/aztec-packages@bdd9b06))
* **avm:** Range check gadget
([#7967](AztecProtocol/aztec-packages#7967))
([0dd954e](AztecProtocol/aztec-packages@0dd954e))
* Proof surgery class
([#8236](AztecProtocol/aztec-packages#8236))
([10d7edd](AztecProtocol/aztec-packages@10d7edd))


### Bug Fixes

* **bb-prover:** Create structure for AVM vk
([#8233](AztecProtocol/aztec-packages#8233))
([55b6ba2](AztecProtocol/aztec-packages@55b6ba2))
* **bb:** Mac build
([#8255](AztecProtocol/aztec-packages#8255))
([ac54f5c](AztecProtocol/aztec-packages@ac54f5c))
* Handle constant output for sha256
([#8251](AztecProtocol/aztec-packages#8251))
([0653ba5](AztecProtocol/aztec-packages@0653ba5))


### Miscellaneous

* **avm:** Replace range and cmp with gadgets
([#8164](AztecProtocol/aztec-packages#8164))
([cc12558](AztecProtocol/aztec-packages@cc12558))
* **bb:** Use std::span in pippenger for scalars
([#8269](AztecProtocol/aztec-packages#8269))
([2323cd5](AztecProtocol/aztec-packages@2323cd5))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants