feat: onchain payment method #365
No reviewers
Labels
No labels
breaking change
bug
documentation
enhancement
needs discussion
needs implementation
new nut
ready
wallet-only
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
forgejo-admin/nuts!365
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "onchain"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
One nit suggestion, one clarification question - otherwise LGTM!
What happens if the wallet sends more proofs? Are they melted and sent on to the bitcoin address?
I assume that's problematic as it affects the fee. But we should be clear to prevent inconsistent mint action.
Small nit... it would be super helpful if we kept the base NUT keys in order and first in the list. Then it's more obvious what has been added in the NUT.
Discussed briefly offline:
The multi-quote is quite a significant change to NUT-05. It maybe also adds resource tracking overhead (4-5 quotes per request) and requires new wallet UI to handle quote selection (vs existing methods).
The tradeoff/benefit might be worth it, but perhaps we should consider setting a desired target confirmation speed in the request (eg
target_blocks/priorityetc) and just returning the closest quote so NUT-05 stays "one request, one quote"@ -0,0 +42,4 @@- `expiry` is the Unix timestamp until which the mint quote is valid- `pubkey` is the public key from the request- `amount_paid` is the total confirmed amount paid to the request in UTXOs that are eligible for minting- `amount_issued` is the amount of ecash that has been issued for the given mint quoteI think that is good to return the minimum confirmations from mint's to wallet. The wallet can check it outside before try to check the quote state.
We need to put some security disclosure because the example has a real Bitcoin address, that someone can send funds to it.
Maybe we can change this address to an testnet address, because this specified address is associated with a big scam.
@ -0,0 +42,4 @@- `expiry` is the Unix timestamp until which the mint quote is valid- `pubkey` is the public key from the request- `amount_paid` is the total confirmed amount paid to the request in UTXOs that are eligible for minting- `amount_issued` is the amount of ecash that has been issued for the given mint quoteOBS: That's already implemented on
MintMethodSetting, but i think that's good to return onPostMintQuoteOnchainResponsetoo.https://github.com/cashubtc/nuts/pull/365/changes#diff-f42c146fe5cf1807b0e4ef524f727dcfa12d8e2221480cf58577b5410d478c53R165
Since we has a minimum number of confirmations, the state will update to paid only if reach it.
Good call we should make it a partial address.
is this necessary? its already stated in NUT-20
Also, why not support nut-08? Its nice to not have to do a swap first, but I guess that's abusing change.
nit, but that last sentence is a bit weird to me because the fact that nut08 is not supported is why the mint must not return change
@robwoodgate can you elaborate more on this? My thinking is that after the wallet picks the quote they only have to track that one and forget about the rest
Providing multiple options is pretty standard for onchain UIs. If a wallet doesn't want to offer quote selection to the user, then it can just select the fastest one and leave the UI unchanged.
This will lead to proofs being stuck pending for 10+ min waiting for confirmation. I think this is bad ux so swapping should be forced. It's also much simpler for the mint to not have to deal with change.
A mint has to track/hold all quotes it creates. It just seems wasteful on resources to have multiple quotes per request. Could also lead to a DoS vector.
It's different to the way all other methods are handled. In CTS, for example, I had to write custom handlers vs leaning on the generics. I just don't know as we need it when it's just as simple to send a param with desired confirmation speed up front.
I'd propose for the wallet to ask for a melt quote targeting desired time (estimated blocks) to be confirmed in the
PostMentQuoteOnchainRequestand the mint adjusting and returning a single mint quote based on current fee environment - as an alternative to the mint responding with multiple melt quotes, which would be very specific (and yet options limiting) for this payment method.Architecturally, transaction timing is pure user / wallet concern and I do not see the reason to increase complexity on the mint side if it can be resolved by 1 additional melt request param that exactly fits user expectation.
The mint having to handle any number of blocks is more complex. Most exchanges offer a fast (next block), medium and slow option with the later two being cheaper as your tx gets batched with more txs.
One option is as Rob suggested where the wallet includes a requested conf in the request and the mint gives a quote back for the closest bucket. We would need to define how closest is chosen (round up or down).
The mint could advertise its "buckets" in the mint info. Though I generally dislike over using mint info.
I don't think this is too much of a concern it's really only a couple extra db entry's and there is nothing really to monitor like there is for mint quotes.
This though is a valid point and would be the justification for change over the dos point.
@ -14,6 +14,7 @@ Method-specific NUTs describe how to handle different payment methods. The currenit pick: the plural here would conform better to existing language
I'd propose adding a "
priority" param forPostMeltQuoteOnchainRequest, something along the lines of:where
priorityis one of the following values:L: 11+ blocksM: 3 - 10 blocksH: 1 - 2 blocksA concern I have with this is that I think a wallet UI should be able to show the user the actual fee for each priority option. If the wallet has to pick the priority up front, then the only way I can think to know the actual fee is by creating a quote for each.
I would prefer to just get all the onchain quotes at once -> present them to the user -> fire the melt on a quote being selected.
An alternative approach would be to set a
max_feeparam toPostMeltQuoteOnchainRequest, then the mint can return the "fastest" settlement quote that fits within the user-set maximum.I don't really mind how it is done, but I do think allowing NUT-05 to return multiple quotes is a line we should not cross without careful consideration. Although I've already written the code for CTS, it still seems "wrong" to have this exception.
I'm reminded of ZEN no.8: "Special cases aren't special enough to break the rules."
I did not meant to make the mint handle any blocks, just fee estimate. As it is always an estimate, it could be done both by passing L,M,H in the request or - as I've proposed - to pass acceptable number of blocks to confirm, e.g. 1 or 10. Mint's fee estimation would then calculate the fee. Both are in essence the same, keep user specific concern on the wallet side and do not introduce special structures in the response.
@ -14,6 +14,7 @@ Method-specific NUTs describe how to handle different payment methods. The curreOne way to avoid the change here would be to define the onchain response as having an interior list instead of returning a list of quotes this avoids any change to nut05 wording. But is maybe just working around the spec and not in the spirit of it
This still has the issue gudnuf pointed out of the user does not know the fee before hand for x confs. The user is choosing between 1 or 10 confs based on the cost but if they do not know the cost they have to create two separate network calls to create 2 quotes if they are not returned as a list up front.
addressed in https://github.com/cashubtc/nuts/pull/365/commits/706ef5c474270a53a4ce1c94cd336d934d2f18f7
The new fee_options works for me - best of both worlds. ACK.
@ -0,0 +200,4 @@],"selected_fee_index": <int|null>,"outpoint": <str|null>}nit - could we order the NUT-05 defaults first? makes it easier to see what has been added by the NUT
I think keeping it here does more good than harm. Easy to overlook a warning if it is in another NUT.
A thought on SIG_ALL: onchain melt SIG_ALL currently commits to
inputs || quotebecause there are no NUT-08 change outputs. It does not commit to the selectedestimated_blocks.The exact input amount binds the selected fee amount, BUT if two
fee_optionshave the same fee and differentestimated_blocks, the same signed proofs can authorize either fee option.I'm not sure that distinction matters, but if it does, duplicate fee values should also be forbidden... otherwise
estimated_blockswould have to be included in the signed message, which is making onchain "special" again :/Lets just avoid this and forbid two from having the same fee. There is no reason for two to have the same fee.
Addressed in https://github.com/cashubtc/nuts/pull/365/commits/e62c585648b833a010dc8a94100d05f35df853d0
Addressed in https://github.com/cashubtc/nuts/pull/365/commits/e62c585648b833a010dc8a94100d05f35df853d0
@ -0,0 +200,4 @@],"selected_fee_index": <int|null>,"outpoint": <str|null>}addressed in 208799ca
Think this is addressed in other threads on this PR marking as resolved.
@ -0,0 +42,4 @@- `expiry` is the Unix timestamp until which the mint quote is valid- `pubkey` is the public key from the request- `amount_paid` is the total confirmed amount paid to the request in UTXOs that are eligible for minting- `amount_issued` is the amount of ecash that has been issued for the given mint quoteI think including it in the quote implies it can be different per quote and is not a global config of the mint that should not change much so it can be kept in mint info.
I think this is addressed by https://github.com/cashubtc/nuts/pull/365/commits/cdb5d264cfcce615fefa07ef63e975f747bd7afb so going to mark as resolved.
ACK e62c585648b833a010dc8a94100d05f35df853d0
Resolved in d76e7e55
re-ACK d76e7e55588f53bdc3e24a0ecfd3b4e916f09788
I am rethinking the use of an absolute fee and wondering if we should allow change.
With no ability to give change the mint must be able to fairly accurately as over estimating too much will cause users to complain of high fees and underestimating will cause tx's to fail at melt time when attempting to construct the final one.
I think this problem is exacerbated in payjoin where onchain inputs could change and thus the fee is changed but with a fixed fee up front there is less flexibility that allowing change would provide.
The issue of change being stuck in pending for 10+ min is still valid but this could be made a wallet problem where they should send no more then amount + fee_reserve not that there is no change given.
After CDK dev call seems like this is a good idea and we should allow change
Resolved. It'll be in the mint-info settings. See responses below
How about including the mint's
confirmationsnumber in thePostMintQuoteOnchainResponse, so that the wallet and the end user know how long they might have to wait? A large - but hidden -confirmationsnumber could be a bad UX(I tried to post this comment in line at
XX.md:29, but Github is giving me errors. Hence I'll try this comment)TL/DR: I think the only unambiguous way to give any meaning to 'expiry' is to say that the mint must honour any payment which has achieved sufficient confirmations (e.g. 6) within 2,016 blocks of the expiry. So maybe the 'expiry' should be specified as a block height instead of a time?
'evicted' in
XX.md:47might be too ambiguous a term for a NUT. A transaction may be evicted from one node's mempool, but not from another node's mempool, and it could still be confirmed even after being evicted from every Bitcoin Core mempool.Once a transaction is seen by anyone, the only thing that can stop it being confirmed is if at least one of its inputs is spent in another block (where that block has multiple confirmations). Until that 'double spend' happens, the transaction remains 'confirmable' for the indefinite future.
Ultimately, I don't think we can specify anything related to the mempool.
The only reason to even have an expiry is to free the mint from having to permanently monitor an arbitrarily large set of addresses. Therefore, I think we say that a payment either had sufficient confirmations before a deadline, or it was too late and the mint is free to ignore further payments to that address
(Again, Github is giving me an "Failed to save comment: An internal error occurred, please try again.." error if I try to make the inline comment in the right place)
Its come up a few times that it may benefit wallet ux to have an amount_incoming value that is the amount the mint sees in the mempool or onchain that has not reached the required number of confs.
This would increase complexity of the mint having to mange mempool tx's that could be dropped from the mempool or removed via RBF.
Also some node backends or services may not provide a mempool so the mint does not have this information. For example a mint that is using compact block filters does not have a mempool.
Given this I lean towards not adding this, as we cannot guarantee mints can support it and I think providing it optionally is worse then not at all and it is up to wallets to use a mempool explorer if they want to show pending.
Also having issues with inline comments.
While minting: Are
units other thansatandmsatmeaningful?If yes, and
usdis meaningful, then maybe the mint should be required to return an exchange rate inPostMintQuoteOnchainResponse?And I guess a similar question apples to melting, but I haven't read that section in detail yet
This is in the mint info, and its up to the wallet how to display this. https://github.com/cashubtc/nuts/pull/365#discussion_r3195422747
Nice. Sorry I missed that:
How about adding a few words to refer to NUT-04 and make this more explicit? I guess we can assume that mint developers would know this; but wallet devs mightn't know exactly where to check. Something like:
ACK @ 700cf7934ad96359c85b749e23fb1a96b0fbfd17. LGTM!
@robwoodgate reword prefer async as we discussed https://github.com/cashubtc/nuts/pull/365/commits/cd2132b4bd5adf7b7bd1334847ddd0d4f5bd0966
Looks great to me - ACK
We need to define all fields on request/response structs, even if is optional
ACK
53a7fea28e@ -0,0 +1,408 @@# NUT-30: Payment Method: Onchain@ -0,0 +196,4 @@"fee_index": <int>,"fee_reserve": <int>,"estimated_blocks": <int>}@ -0,0 +1,408 @@# NUT-30: Payment Method: Onchain@ -0,0 +247,4 @@"fee_index": <int>,"fee_reserve": <int>,"estimated_blocks": <int>}@ -0,0 +342,4 @@"fee_index": 2,"fee_reserve": 800,"estimated_blocks": 144}@ -0,0 +376,4 @@"fee_index": 2,"fee_reserve": 800,"estimated_blocks": 144}