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

Payment Method Refactor #201

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

davidcaseria
Copy link

No description provided.

Copy link

@purpleKarrot purpleKarrot left a comment

Choose a reason for hiding this comment

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

I am in favor of the technical language and the fact that specifics to the payment method are no longer part of the Request and Resonse type names.

"unit": <str_enum["sat"]>,
"description": <str|null>
"unit": <str>,
"opts": <obj|null>

Choose a reason for hiding this comment

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

I am in favor of the change, but strictly speaking it is a breaking change.


```bash
curl -X POST http://localhost:3338/v1/mint/quote/bolt11 -d '{"amount": 10, "unit": "sat"}' -H "Content-Type: application/json"
curl -X POST https://mint.host:3338/v1/mint/quote/{method} \

Choose a reason for hiding this comment

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

Here, using bolt11 should be preferred over {method}, because it is an example and other variables have concrete values too (10 and sat).


```bash
curl -X GET http://localhost:3338/v1/mint/quote/bolt11/DSGLX9kevM...
curl -X GET https://mint.host:3338/v1/mint/quote/{method}/{quote_id}

Choose a reason for hiding this comment

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

Again, use concrete values in an example.


```bash
curl -X POST https://mint.host:3338/v1/mint/bolt11 -H "Content-Type: application/json" -d \
'{
curl -X POST https://mint.host:3338/v1/mint/{method} \

Choose a reason for hiding this comment

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

Here too.


```bash
curl -X POST https://mint.host:3338/v1/melt/quote/bolt11 -d \
{
curl -X POST https://mint.host:3338/v1/melt/quote/{method} -d \

Choose a reason for hiding this comment

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

Here too.


```bash
curl -X GET http://localhost:3338/v1/melt/quote/bolt11/TRmjduhIsPxd...
curl -X GET https://mint.host:3338/v1/melt/quote/{method}/{quote_id}

Choose a reason for hiding this comment

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

Here too.


```bash
curl -X POST https://mint.host:3338/v1/melt/bolt11 -d \
curl -X POST https://mint.host:3338/v1/melt/{method} -d \

Choose a reason for hiding this comment

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

Here too.


```json
{
"quote": <str>,
"inputs": <Array[Proof]>,
"outputs": <Array[BlindedMessage]> <-- New
"outputs": <Array[BlindedMessage]>

Choose a reason for hiding this comment

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

What is the rationale for dropping the <-- New comment?

"payment_preimage": <str|null>,
"change": <Array[BlindSignature]> <-- New
"change": <Array[BlindSignature]>

Choose a reason for hiding this comment

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

Also, why remove the comment?


```bash
curl -X POST https://mint.host:3338/v1/melt/bolt11 -d \
curl -X POST https://mint.host:3338/v1/melt/{method} -d \

Choose a reason for hiding this comment

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

Again, use concrete values in examples.

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