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!: move kernel MMR position to u64 #5956

Merged

Conversation

AaronFeickert
Copy link
Collaborator

Description

Switches the handling of kernel MMR positions from u32 to u64.

Closes #5924.

Motivation and Context

Internally, MMR positions are handled as either usize or u64. However, they are eventually parsed and handled as u32 for storage. While it doesn't address the internal usize handling, it switches storage operations to be u64.

How Has This Been Tested?

Existing tests pass.

What process can a PR reviewer use to test or verify this change?

Assert that the logic, especially around LMDB operations, is correct.

BREAKING CHANGE: Modifies the handling of kernel MMR data in LMDB.

@AaronFeickert AaronFeickert changed the title feat! move kernel MMR position to u64 feat!: move kernel MMR position to u64 Nov 14, 2023
@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Nov 14, 2023
@AaronFeickert AaronFeickert marked this pull request as ready for review November 14, 2023 19:08
Copy link

Test Results (CI)

1 258 tests   1 258 ✔️  10m 58s ⏱️
     39 suites         0 💤
       1 files           0

Results for commit f463bdd.

Copy link

Test Results (Integration tests)

31 tests   31 ✔️  14m 41s ⏱️
11 suites    0 💤
  2 files      0

Results for commit f463bdd.

Copy link
Collaborator

@SWvheerden SWvheerden left a comment

Choose a reason for hiding this comment

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

I tested this locally running a new blockchain, an syncing a new block chain. Works 100%

@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Nov 16, 2023
@SWvheerden SWvheerden merged commit cdd8a31 into tari-project:development Nov 16, 2023
14 of 15 checks passed
@AaronFeickert AaronFeickert deleted the kernel-mmr-position branch November 16, 2023 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MMR use position as u32
3 participants