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

[Feature] Support historical logging for delegated, bonded, and unbonding` mappings. #2493

Merged

Conversation

d0cd
Copy link
Contributor

@d0cd d0cd commented Jun 12, 2024

This PR supports historical logging for the delegated, bonded, and unbonding mappings in credits.aleo.

If the history feature is enabled, the VM will store the above mappings as JSON to a set local directory (the same directory as the .ledger).

This data is stored and updated as follows:

  • A top-level .history* directory is created at the storage path.
  • When a block i is added to the ledger, a subdirectory block-<i> is created containing: block-<i>-delegated.json, block-<i>-bonded.json, and block-<i>-unbonding.json.
  • If the history directory is deleted, it is created once again.
  • Data is stored as a JSON string for easier consumption. We considered storing it at bytes, but opted for usability over compression.

Pruning and management of the .history directory is left to the node runner.

Note. The entirety of this PR is gated under the history feature.

CI passed mostly here. The failing tests are succeeding locally and unrelated to this PR.

@d0cd d0cd requested a review from vicsn June 13, 2024 16:43
@vicsn vicsn requested a review from apruden2008 June 13, 2024 16:52
vicsn
vicsn previously approved these changes Jun 13, 2024
Copy link
Contributor

@vicsn vicsn left a comment

Choose a reason for hiding this comment

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

LGTM!

As for CI (thx for pushing this already!):
Can you also push a clippy fix? I assume maybe a new Rust version came out.
Can you also run the synthesizer tests locally to double check they pass?

Meshiest
Meshiest previously approved these changes Jun 13, 2024
Copy link

@Meshiest Meshiest left a comment

Choose a reason for hiding this comment

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

LGTM. We use a tool that uses a similar mechanism that captures the raw binary values of all mappings and they are quite compact (under 4-16KB per block)

@zosorock zosorock requested review from evanmarshall, ljedrz, niklaslong and a team June 14, 2024 16:27
@d0cd d0cd closed this Jun 14, 2024
@d0cd d0cd reopened this Jun 14, 2024
@d0cd
Copy link
Contributor Author

d0cd commented Jun 14, 2024

LGTM. We use a tool that uses a similar mechanism that captures the raw binary values of all mappings and they are quite compact (under 4-16KB per block)

Do you have opinions on whether the data should be stored as raw JSON strings or as bytes?

@apruden2008
Copy link
Contributor

We ran our own internal CI against this and is passing. Given the fact this is gated behind a CLI flag, I think this is a safe feature to add in here at this point in the development timeline. Thank you @Meshiest and @damons for your review

@apruden2008 apruden2008 merged commit 6d64025 into AleoNet:mainnet-staging Jun 14, 2024
9 of 83 checks passed
Copy link
Contributor

@apruden2008 apruden2008 left a comment

Choose a reason for hiding this comment

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

See comment above. LGTM based on internal CI passing and prior reviews both inside and outside of Provable

@HarukaMa
Copy link
Contributor

I just saw this and I'd highly recommend anyone who use this to store the jsons in a fs that supports compression, since with a busy network those data could get huge really fast. Had to manually implement a similar function to debug my explorer before. Also one of the reasons I don't keep a whole history of bonded and deletaged mappings - they are really huge on a live network.

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