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

Slatepack OwnerAPI Changes #421

Merged
merged 8 commits into from
May 28, 2020
Merged

Conversation

yeastplume
Copy link
Member

@yeastplume yeastplume commented May 27, 2020

This PR aims to:

  • Expose Slatepack changes via the OwnerAPI
  • Ensure all user-facing instance of addresses in the API use a SlatepackAddress. This means that what was previously either being input/output by the user as a payment proof address or TOR address will now be a Slatepack address.

In detail:

  • Change the get_public_proof_address Owner API function to get_slatepack_address and return a slatepack address instead.
  • Remove the proof_address_from_onionv3 function from the Owner API as it's no longer needed
  • As a result of above changes, arguments to init_send_tx and other places become a slatepack address.
  • Add a new Owner API functions to output a slate as a Slatepack

New API functions are:

  • create_slatepack_message - returns an (optionally encrypted) armored slatepack string from a slate
  • slate_from_slatepack_message - extracts the slate from the given slatepack string, attempting to decrypt from a range of deriviation indices if necessary (takes a vec instead of a single value for future multiple-address use)
  • decode_slatepack_message - convenience function that returns the Slatepack struct itself, which can be viewed as json.

Notes:

  • The command line might be a bit broken as of this PR, which is fine and will be addressed in the next PR which will implement the command-line slatepack workflow.
  • Internally, addresses are still stored in the tx log and appear in the slate as ed25519 keys, for backwards compatibility.

@yeastplume yeastplume changed the title [WIP] Slatepack OwnerAPI Changes Slatepack OwnerAPI Changes May 27, 2020
@yeastplume
Copy link
Member Author

Updated top comment, should be ready for review

@yeastplume yeastplume requested a review from j01tz May 27, 2020 21:31
Copy link
Member

@j01tz j01tz left a comment

Choose a reason for hiding this comment

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

Reviewed for general slatepack logic in OwnerAPI, a few non-critical notes:

  • is it worth splitting out get_slatepack_secret_edkey() and get_slatepack_secret_xkey() in the API in case someone wants to do encryption/decryption at a different layer? or is that too much complexity/opportunity for confusion in the API calls for the convenience?

  • before beta we will want to add stricter validation for anything exposed in libwallet

This is coming together nicely, can't wait to start hitting the command-line workflow.

I hope some others are watching this to see beastplume in action

@yeastplume
Copy link
Member Author

Reviewed for general slatepack logic in OwnerAPI, a few non-critical notes:

  • is it worth splitting out get_slatepack_secret_edkey() and get_slatepack_secret_xkey() in the API in case someone wants to do encryption/decryption at a different layer? or is that too much complexity/opportunity for confusion in the API calls for the convenience?

This can be added at any time if anyone calls for it, but might as well keep it simple for the moment.

@yeastplume yeastplume merged commit db12712 into mimblewimble:master May 28, 2020
@yeastplume yeastplume deleted the slatepack_api branch July 13, 2020 10:20
antiochp pushed a commit to antiochp/grin-wallet that referenced this pull request Aug 7, 2020
* change all user-facing instances of addresses to a SlatepackAddress

* finish renaming get_slatepack_address + documentation

* get_slatepack_secret_key OwnerRPC implementation and test

* add owner api functions

* OwnerRPC functions + doctests

* add explicit slatepack API tests to exercise encryption

* update api function names to better reflect RFC
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.

None yet

2 participants