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

Initial signer sync #329

Merged
merged 36 commits into from
Mar 28, 2023
Merged

Initial signer sync #329

merged 36 commits into from
Mar 28, 2023

Conversation

SupremoUGH
Copy link
Contributor

@SupremoUGH SupremoUGH commented Mar 14, 2023

Goes together with:

Implements the Initial Synchronization method for the signer. For that, there's a new method that creates a partial Merkle tree from a vector of leaves and a path, extended to forked trees and forests.
On the wallet side, we read the relevant data from the ledger and then use it to update the signer, without any Poseidon hash computation (as opposed to the usual sync method).

Design choice:

  • I chose to instantiate the MK from a path instead of a SinglePath MK. Using the second, we could check the roots of coincide, making it more secure, and less trusting on the pulled data. However, I think the data pulled from the Ledger is considered trusted, so no need for additional computations.

On the testing/simulation: Besides the new unit tests for the partial and forked MKs, I ran the simulation, kept the Ledger, and then re-ran it again with new actors which would do the initial sync w.r.t. the new ledger. It all went smoothly, as expected. The code was quite clumsy and it'd take a long time to make it look better (and add nice features like new actors joining the simulation at arbitrary times) so I removed that part, but it's good to know that it worked as expected.


Before we can merge this PR, please make sure that all the following items have been checked off:

  • Linked to an issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Added one line describing your change in CHANGELOG.md and added the appropriate changelog label to the PR.
  • Re-reviewed Files changed in the GitHub PR explorer.
  • Checked that changes and commits conform to the standards outlined in CONTRIBUTING.md.

@SupremoUGH SupremoUGH changed the title checkpoint Initial signer sync Mar 15, 2023
@SupremoUGH SupremoUGH marked this pull request as ready for review March 22, 2023 16:50
@SupremoUGH SupremoUGH self-assigned this Mar 22, 2023
@SupremoUGH SupremoUGH added the changelog:added Changelog: add these changes to the `added` section of the changelog label Mar 22, 2023
manta-accounting/src/wallet/mod.rs Outdated Show resolved Hide resolved
manta-accounting/src/wallet/mod.rs Outdated Show resolved Hide resolved
manta-accounting/src/wallet/signer/mod.rs Outdated Show resolved Hide resolved
manta-accounting/src/wallet/signer/mod.rs Outdated Show resolved Hide resolved
manta-crypto/src/accumulator.rs Outdated Show resolved Hide resolved
manta-crypto/src/merkle_tree/forest.rs Outdated Show resolved Hide resolved
@SupremoUGH SupremoUGH requested a review from bhgomes March 24, 2023 11:41
Copy link
Contributor

@Apokalip Apokalip left a comment

Choose a reason for hiding this comment

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

lgmt

@bhgomes bhgomes merged commit f6606ca into main Mar 28, 2023
@bhgomes bhgomes deleted the signer_initial_sync_checkout branch March 28, 2023 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:added Changelog: add these changes to the `added` section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants