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: adding a simple masm wallet an eoa #233

Merged
merged 8 commits into from
Sep 13, 2023

Conversation

Dominik1999
Copy link
Collaborator

Closes #22 and is a new version of #213

This PR implements a basic wallet interface. For now, we only have receive_asset, send_asset.

  • Tests are working and test basic functionality.

    • The first test tests receive_asset and the account in fact has the asset in its vault after the transaction
    • The second test send_asset and the account doesn't have the asset in its vault after the transaction [Note: here is missing that we don't actually have a note at the end of the test, the note is stored in memory of the account]
  • Also the basic eoa auth_tx procedure only increases the nonce. The Falcon signature verification is not there yet. @Al-Kindi-0

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! Not a full review as I've only looked through MASM code - but I left a few small comments inline.

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.

Thank you! Looks good! I left a few comments inline. The only significant one is about potentially moving the tests into miden-tx crate.

@Dominik1999 Dominik1999 force-pushed the dominik_add_basic_asm_wallet_new_try branch from 51ef703 to 229c173 Compare September 12, 2023 12:40
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.

All looks good! Thank you! I left a couple of nits inline. Once these are addressed and we rebase from #236 (I think @hackaugusto is almost done with it), it is good to go.

@Dominik1999 Dominik1999 merged commit c954b3b into main Sep 13, 2023
@Dominik1999 Dominik1999 deleted the dominik_add_basic_asm_wallet_new_try branch September 13, 2023 12:18
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.

Implement basic wallet interface
4 participants