Conversation
Semisol
left a comment
There was a problem hiding this comment.
This finally got proposed :D
|
LGTM |
There was a problem hiding this comment.
ACK 16e0a39.
I pointed a few non-blocking nits if you're interested.
Other ramblings/thoughts
A general thought on the wording of those token states (which might belong more in an issue than on this PR so feel free to ignore). It makes sense to have booleans in this json object for communication between the mint and client, but it might be nice to outline better wording for those states than the negation of spendable and pending as not spendable and not pending. In my mind, those two booleans immediately combine to become an enum, and I'm likely to call the enum variants something like spendable/burned/inflight or live/burned/pending.
This is more of an implementation detail (the json fields should probably stay spendable/pending) but as outlined in this awesome and very liked PR from Murch, providing good wording is always a big win. I guess what I'm proposing here is another PR that clarifies what the spec will call these token states. Something like this or a variant of it with your preferred words:
We call the combination of these booleans the token's state. A token can either be live (spendable == true && pending == false), in-flight (spendable == true && pending == true), or burned (spendable == false && pending == (true || false)).
This is necessary in order for a wallet to determine whether a token it used for making a Lightning payment is still in-flight or not.
In short, LN payments can have 3 states but our tokens only 2 (because there's only
spendableor not). We need another boolean state (pending).Using this, wallets can determine the state of an outgoing Lightning payment as follows:
not spendableandnot pending, the payment succeededspendableandpending, the payment is in-flightspendableandnot pending, the payment failed