Skip to content
This repository has been archived by the owner on Dec 12, 2024. It is now read-only.

Clarify Offering requiredPaymentDetails undefined #227

Merged
merged 3 commits into from
Feb 20, 2024

Conversation

diehuxx
Copy link

@diehuxx diehuxx commented Jan 19, 2024

No description provided.

@@ -115,7 +115,7 @@ An `Offering` is used by the PFI to describe a currency pair they have to _offer
| field | data type | required | description |
|--------------------------|-----------------------------------------|----------|---------------------------------------------------------------------------------------------------|
| `kind` | string | Y | Type of payment method (i.e. `DEBIT_CARD`, `BITCOIN_ADDRESS`, `SQUARE_PAY`) |
| `requiredPaymentDetails` | [JSON Schema](https://json-schema.org/) | N | A JSON Schema containing the fields that need to be collected in order to use this payment method |
| `requiredPaymentDetails` | [JSON Schema](https://json-schema.org/) | N | A JSON Schema containing the fields that need to be collected in the RFQ's selected payment methods in order to use this payment method. If `requiredPaymentDetails` is blank, then the RFQ's selected payment method will always be accepted. |
Copy link
Contributor

Choose a reason for hiding this comment

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

this feels a bit odd to me. does this mean that Alice can send in anything in her payin payment method and the PFI will accept it? like, Alice can buy bitcoin with cheese?

Copy link
Author

Choose a reason for hiding this comment

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

like, Alice can buy bitcoin with cheese?

Youre describing my hopes and dreams.

does this mean that Alice can send in anything in her payin payment method

In my opinion, yes Alice can send anything in the payment details. If the Offering does not have any required payment details, there are no requirements on the payment details contained in Alice's RFQ. If the PFI does not want to accept cheese as payment, they better go to another farmers market specify that in their Offering.

In practice, Alice would realistically just leave the payment details empty if there were no required payment details.

Is there a different behavior that you would prefer for absent requiredPaymentDetails?

Copy link
Contributor

@jiyoonie9 jiyoonie9 Jan 25, 2024

Choose a reason for hiding this comment

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

blue cheese for bitcoin is a fair trade tbh. they're both funky and not everyone likes them!

anyways, back to the main point - mostly i am not sure why requiredPaymentDetails is allowed to be optional (and i think it was optional before you created this PR). Shouldn't the PFI always have some preferred method of payin payment they accept for what they're paying out?

another question would be, does the lack of this field also imply that Alice can get bitcoin without paying the PFI anything?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it implies that Alice does not have the pay the PFI if the PFI does not require it. It is up to the PFI.

Copy link
Author

Choose a reason for hiding this comment

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

Update from face to face chat: We decided that it's better to require that paymentDetails be omitted if requiredPaymentDetails is omitted, rather than place no restrictions on paymentDetails.

specs/protocol/README.md Outdated Show resolved Hide resolved
@diehuxx diehuxx force-pushed the required-payment-details branch from f04d3a1 to ce1d834 Compare February 13, 2024 23:55
@diehuxx diehuxx merged commit 9a9ed69 into main Feb 20, 2024
@diehuxx diehuxx deleted the required-payment-details branch February 20, 2024 18:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants