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

Add memo to transaction creation #244

Merged
merged 2 commits into from
Jun 16, 2022
Merged

Add memo to transaction creation #244

merged 2 commits into from
Jun 16, 2022

Conversation

MartinMinkov
Copy link
Contributor

@MartinMinkov MartinMinkov commented Jun 13, 2022

Description
Adds the memo property to various methods in SnarkyJS. The following methods now have a memo property:

  1. signFeePayer
  2. addFeePayer
  3. Mina.transaction
  4. Deploy

These changes allow attaching a memo to a Parties structure. If there is no memo passed into these methods, they will simply reuse the memo property in the Parties structure.

All the code to create a signature with the memo field is already included in the backend OCaml code. It just needed to be exposed in SnarkyJS.

Tested By
Manual testing

@mitschabaude
Copy link
Collaborator

@MartinMinkov This is a good addition, but doesn't yet expose the memo field to the most important user API for creating transactions, which is Mina.transaction. It would be cool to also have an optional memo field in the (optional) first argument to Mina.transaction

@MartinMinkov MartinMinkov force-pushed the feat/add-memo-to-txn branch from 273e09a to c8b3afc Compare June 13, 2022 18:23
Copy link
Collaborator

@mitschabaude mitschabaude left a comment

Choose a reason for hiding this comment

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

LGTM!

@mitschabaude mitschabaude merged commit 55223d8 into main Jun 16, 2022
@mitschabaude mitschabaude deleted the feat/add-memo-to-txn branch June 30, 2022 07:52
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.

2 participants