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

refactor(StateVariables): Change max_block_number to max_timestamp #5757

Open
Tracked by #4761
nventuro opened this issue Apr 15, 2024 · 3 comments
Open
Tracked by #4761

refactor(StateVariables): Change max_block_number to max_timestamp #5757

nventuro opened this issue Apr 15, 2024 · 3 comments
Assignees

Comments

@nventuro
Copy link
Contributor

The max_block_number property is used to constrain things like SharedMutable (#5490), but choosing delays in blocks is both awkward and prone to issues if the block time ever changes. It'd be much better if we could have these work with actual timestamps.

A problem with using timestamps would be that if block production were to suddenly halt, transactions would become invalidated due to having expired, which would not occur with block numbers. This may or may not be an issue.

The change itself is simple to make, since the entire structure is already there - it's simply a matter of changing the assertions and initializations to use time instead of block numbers.

@nventuro nventuro added the discussion-needed Still needs more discussion before we jump to work on this label Apr 19, 2024
@nventuro
Copy link
Contributor Author

Another issue with using block numbers is that we're unable to magically change this value for testing purposes - we must actually mine full blocks, which is quite slow. This makes us want to have short delays (e.g. 3-5 blocks). But it's also error-prone if tests need to send more transactions, potentially unintentionally pushing past the block of change.

Changing the time on the other hand is instant and cannot result in accidental errors.

@nventuro
Copy link
Contributor Author

A problem with using timestamps would be that if block production were to suddenly halt, transactions would become invalidated due to having expired, which would not occur with block numbers. This may or may not be an issue.

Using blocks, a halted network does not move any of the delayed actions forward, whereas using timestamps it'd be possible to have a value change to through without actors having the opportunity to react to this. I'd argue however that this sounds like a network issue and not an application one.

@LHerskind LHerskind changed the title Change max_block_number to max_timestamp refactor(StateVariables): Change max_block_number to max_timestamp Jun 11, 2024
@LHerskind LHerskind removed the discussion-needed Still needs more discussion before we jump to work on this label Jun 11, 2024
@nventuro nventuro added this to the aztec-nr - 1.0 milestone Jun 13, 2024
@nventuro
Copy link
Contributor Author

max_timestamp doesn't seem to be a well-liked name, we'll go with include_by_timestamp instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

2 participants