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

Command line implementation of invoice commands #96

Merged
merged 10 commits into from May 9, 2019

Conversation

Projects
None yet
4 participants
@yeastplume
Copy link
Member

commented May 2, 2019

Following from #90, the actual command line implementation exercising the invoice commands.

The code will mostly be a lot of boilerplate copying from the parsing and api execution system that's in place, but the particular file to watch for comments on the UI (if you can even call it that) is the command line arg yaml definition file:

src/bin/grin-wallet.yml

  • invoice command
  • pay command
  • finalize command
  • community review, particularly on naming, arguments, and process_invoice workflow
  • test implementation exercising new commands (ongoing)
@yeastplume

This comment has been minimized.

Copy link
Member Author

commented May 2, 2019

Added issue_invoice command as a start. The process_invoice command is next, and it's likely to be the most difficult due to the need for interaction.

@mcdallas

This comment has been minimized.

Copy link
Contributor

commented May 4, 2019

I would propose changing issue_invoice to invoice (as in the verb), process_invoice to pay and maybe use the existing finalize.

Also it would be nice if by default the invoice would be printed to stdout as a base64 string (instead of file) like in the Lightning network. The workflow I am thinking is:

  • You checkout from a site, they give you a base64 string plus a textbox to paste your reply.
  • User copies the string and types in his wallet grin-wallet pay <string>
  • "Do you want to pay X for Y amount (yes/no)"
  • Wallet prints another string
  • User pastes it to the textbox and clicks submit
@DavidBurkett

This comment has been minimized.

Copy link

commented May 4, 2019

I like @mcdallas's suggestion. Seems like a simple enough workflow for most users, and the wording makes sense to me.

@lehnberg

This comment has been minimized.

Copy link
Collaborator

commented May 5, 2019

👍

@yeastplume

This comment has been minimized.

Copy link
Member Author

commented May 5, 2019

Fine with all of this, except using the same finalize as the regular send case, as the rest of the code keeps them separate throughout. Will make changes

yeastplume added some commits May 7, 2019

@yeastplume yeastplume changed the title [WIP] Command line implementation of invoice commands Command line implementation of invoice commands May 9, 2019

@yeastplume

This comment has been minimized.

Copy link
Member Author

commented May 9, 2019

Think this is just about there, couple of points:

  • I went with reusing the same finalize command and have the command determine whether the Slate is an invoice or not.
  • Will leave the base 64 stuff as a future enhancement. If we start outputting base64 we'd want it consistent across all wallet commands, so it most likely deserves its own thinking and PR.

The confirmation prompting is also in place, so if anyone could look at that and give feedback that would be great. We should have a chance to exercise it for a 1.1.0 beta anyhow

@yeastplume

This comment has been minimized.

Copy link
Member Author

commented May 9, 2019

Merging to allow builds for PR work, sure we can still review and comment on the beta build

@yeastplume yeastplume merged commit 6f875c5 into mimblewimble:master May 9, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@yeastplume yeastplume deleted the yeastplume:invoice_cmd branch May 20, 2019

garyyu added a commit to gottstech/grin-wallet that referenced this pull request May 26, 2019

merge from upstream (#21)
* V2 slate versioning enablement (mimblewimble#85)

* fix for command line listener port override

* reduce parameter query size

* Add slate versioning

* rustfmt

* bump version number

* Add tests for slate version conversion

* rustfmt

* Updates and test addition based on bdap's review

* rustfmt

* fix mimblewimble#88 (mimblewimble#89)

* Add `check_version` function to Foreign API (mimblewimble#87)

* move api deser types into separate types mod

* rustfmt

* missing types file

* make all exports from libwallet more explicit

* rustfmt

* add version check function to foreign api

* rustfmt

* change check_version return value to result, for consistency

* Revert "Merge branch 'master' into master"

This reverts commit a63f2c7, reversing
changes made to d774272.

* [WIP] Invoiced Transactions API Support (mimblewimble#90)

* basic invoiced tx working

* rustfmt

* teardown

* rustfmt

* rename, new struct for invoice args, begin to add new functions to RPC apis

* rustfmt

* add fns to rpc api

* rustfmt

* owner api functions RPC documentation in place

* rustfmt

* doctests for new invoicing functions

* rustfmt

* test fixes

* update documentation and doctests

* rustfmt

* invoice testing verification of tx log output

* rustfmt

* Add Azure Pipelines to grin-wallet

* Remove cursive and fix task names

* Do not crash on empty terminal

* Add LLVM

* Possible MSFT pipeline bug with . task

* Change connection name

* Command line implementation of invoice commands (mimblewimble#96)

* add issue_invoice_tx command

* rustfmt

* add first pass at process_invoice command

* start of process_invoice fn

* rustfmt

* rename issue invoice and process invoice to invoice and pay

* add prompting and display information to pay invoice command

* rustfmt

* support invoice transactions in finalize command

* rustfmt

* Add Azure Pipelines badge

* Remove duplicate LMDB wallet backend code (mimblewimble#101)

* remove leftover wallet lmdb code

* rustfmt

* Simplify slate (de)serialization (mimblewimble#103)

* Simplify slate (de)serialization

* rustfmt

* Cleanup

* Fix slate version tests

* Another fix for tests

* Fix slate deser in http adapter

* docstring update and provide a bit of a readme (mimblewimble#104)

* Updates for 1.1.0-beta.3 release (mimblewimble#106)

* bump version number and build from tag for beta 3 release

* update for latest grin tag

* Fix keybase adapter (mimblewimble#107)

* fix version string in yml (mimblewimble#113)

* fix: invoice transaction pay need update the slate height (mimblewimble#115)

* Remove double json encoding on http response (mimblewimble#114)

* Add participant ID as part of key to stored private transaction context data (mimblewimble#117)

* add participant_id to saved tranasction context data

* rustfmt?

* change participant id for command line pay command

* change the api repository to grin-wallet instead of grin (mimblewimble#118)

* rustfmt

* fix the test

* fix controller and api test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.