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

Cardano transaction sets & proof in client cli #1501

Merged
merged 20 commits into from
Feb 13, 2024

Conversation

Alenar
Copy link
Collaborator

@Alenar Alenar commented Feb 13, 2024

Content

This PR implement several new commands related to the cardano transaction sets & proof in the mithril-client-cli:

  • transactions sets list command
  • transactions sets show <artifact_id> command? (if needed)
  • transactions sets latest command
  • transactions certify <tx1>,<tx2>,...,<txn> command

Those commands are behind a unstable flag that must be provided in order to activate them when running the cli.

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested
  • Documentation
    • Update README file (if relevant)
    • Update documentation website (if relevant)
    • Add dev blog post (if relevant)

Comments

A problem in the transaction proof generation was detected: the order of the transactions used to generated the Merkel Root was not the same as the one used by the signable builder. The signable builder used the transactions by chronological order, the prover used them ordered by transaction hash reversed. We aligned the prover to the signable builder in order to always use the same order.

This PR also remove the dependency to mithril-common in the mithril-client-cli, instead it use the re-exported types from mithril-client library.

Issue(s)

Relates to #1469

Copy link

github-actions bot commented Feb 13, 2024

Test Results

  3 files  ±0   40 suites  +3   8m 29s ⏱️ +4s
830 tests +1  830 ✅ +1  0 💤 ±0  0 ❌ ±0 
917 runs  +3  917 ✅ +3  0 💤 ±0  0 ❌ ±0 

Results for commit d393ddc. ± Comparison against base commit 3a1992f.

♻️ This comment has been updated with latest results.

@Alenar Alenar force-pushed the ensemble/1469/cardano-tx-in-client-cli branch from f4aeebb to 21705fc Compare February 13, 2024 13:26
Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

LGTM 👍

I commented for few minor typos.

@Alenar Alenar force-pushed the ensemble/1469/cardano-tx-in-client-cli branch from 658e162 to 8e2cf21 Compare February 13, 2024 15:53
@Alenar Alenar force-pushed the ensemble/1469/cardano-tx-in-client-cli branch from 8e2cf21 to f5733b6 Compare February 13, 2024 15:59
@Alenar Alenar force-pushed the ensemble/1469/cardano-tx-in-client-cli branch from f5733b6 to d393ddc Compare February 13, 2024 16:30
@Alenar Alenar merged commit 80ee079 into main Feb 13, 2024
40 checks passed
@Alenar Alenar deleted the ensemble/1469/cardano-tx-in-client-cli branch February 13, 2024 16:54
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.

4 participants