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

Added transaction number limit and updated tests to the limits introduced in miden-objects #503

Merged
merged 5 commits into from
Sep 27, 2024

Conversation

polydez
Copy link
Contributor

@polydez polydez commented Sep 23, 2024

Resolves issue 0xPolygonMiden/miden-base#398

Here we use the latest miden-objects with updated/new limits. Also implemented checking for limits we can't check in miden-objects. And also updated tests to the latest miden-object and added at least one asset in note mockups.

crates/block-producer/src/errors.rs Outdated Show resolved Hide resolved
CHANGELOG.md Outdated
@@ -7,6 +7,7 @@
- Optimized state synchronizations by removing unnecessary fetching and parsing of note details (#462).
- [BREAKING] Changed `GetAccountDetailsResponse` field to `details` (#481).
- Improve `--version` by adding build metadata (#495).
- [BREAKING] Introduced additional limits for note/asset/transaction number (#503).
Copy link
Contributor

Choose a reason for hiding this comment

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

The note and transaction limits are enforced in the batch builder, but where is the asset limit checked?

@bobbinth
Copy link
Contributor

Not a full review, but as I mentioned in the miden-base PR, what we want to limit is not the number of transactions per batch/block, but rather the number of updated accounts.

A nice side effect of this should be an incentive to put transactions which touch the same account into a batch/block.

Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you!

@bobbinth bobbinth merged commit 10df0e2 into next Sep 27, 2024
8 checks passed
@bobbinth bobbinth deleted the polydez-limits branch September 27, 2024 03:53
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.

3 participants