cashubtc / nuts

Cashu protocol specifications https://cashubtc.github.io/nuts/
MIT License
152 stars 52 forks source link

NUT-06: Deprecate amount field on PostSplitRequest #32

Closed gandlafbtc closed 1 year ago

gandlafbtc commented 1 year ago
{
  "proofs": Proofs,
  "outputs": BlindedMessages,
  "amount": int //deprecate
}

the amount field is not necessary, since it introduces unwanted constraints. The client has to manage split amounts (send, return) , but the mint does not need to. The mint only needs to perform split according to the clients request.

gandlafbtc commented 1 year ago

For the PostSplitResponse:

class PostSplitResponse(BaseModel):
    fst: BlindedSignatures
    snd: BlindedSignatures

will become

class PostSplitResponse(BaseModel):
    promises: BlindedSignatures

and it is up to the client software to accumulate the proofs accordingly after the split.

thesimplekid commented 1 year ago

This makes sense to me. I was just running into an issue where cashu.me sends outputs in the reverse order cashu-rs expects. This would solve that.

callebtc commented 1 year ago

This makes sense to me. I was just running into an issue where cashu.me sends outputs in the reverse order cashu-rs expects. This would solve that.

Could you explain how this happens? The order shouldn't be expected anywhere specifically iirc (except for the spendable check?)

gandlafbtc commented 1 year ago

the order matters inside the split operation: the second amount is the "send" amount, the frist amount is "return" amount.

But yes, after we upgrade this, it should no longer matter

callebtc commented 1 year ago

the order matters inside the split operation: the second amount is the "send" amount, the frist amount is "return amount"

The fields are labeled though, so there is no "order" between fst and snd. There is an order inside those fields, that's what I thought @thesimplekid is referring to.

thesimplekid commented 1 year ago

The example from NUT-06 where a wallet balance of 64 and the goal to split to a token of 40. The outputs are [32, 8, 16, 8] so the first outputs 32 and 8 in this list, can be combined into the target, a token of 40. However, cashu.me sends the outputs in the reverse order of [8, 16, 8, 32]. Of course the lib can be changed as NUT-06 does not guarantee the ordering of outputs.

https://github.com/clarkmoody/cashu-rs/blob/8560f87862ecece791cb5f7b71b80839bcbe639e/src/mint.rs#L259

        // Create sets of target and change amounts that we're looking for
        // in the outputs (blind messages). As we loop, take from those sets,
        // target amount first.
        for output in split_request.outputs {
            let signed = self.blind_sign(&output)?;

            // Accumulate outputs into the target (send) list
            if target_total + signed.amount <= split_request.amount {
                target_total += signed.amount;
                target.push(signed);
            } else {
                change_total += signed.amount;
                change.push(signed);
            }
        }
thunderbiscuit commented 1 year ago

Concept ACK.

I was re-reading NUT-06 (as part of #26) and came to open a question about this very issue. Wasn't sure if the amount was needed for some corner case I had not thought of, but from what I can tell the outputs will contain all information required to proceed with the split from the mint's perspective.

The exact split amount that the wallet wants is an internal requirement of the wallet which the mint does not need to know about, and in fact it's a small leak of metadata; in the example from the NUT the mint (as it currently stands) knows that what Alice really wanted was to build a 40 sat payment, and not two payments of 8 sats. Removing the amount field removes that extra data.

callebtc commented 1 year ago

ACK.

Implemented in Nutshell mint (and wallet) in this PR: https://github.com/cashubtc/cashu/pull/263

We need to update the NUT now.

callebtc commented 1 year ago

Let's please move the discussion over here and track progress in upgrading: https://github.com/cashubtc/nuts/pull/34