Implement ordinal support - #161
Conversation
816fe7d to
ec82297
Compare
| /// The transaction where the UTXO containing the ordinal is located. | ||
| pub ordinal_tx: Transaction, | ||
| /// Whether it the offer party that should be refunded in case of a contract timeout. | ||
| pub refund_offer: bool, |
There was a problem hiding this comment.
I'm not sure we need this logic. A normal DLC doesn't support giving the refund to the party that didn't lock it initially. Shouldn't this just always go back to who supplied it?
There was a problem hiding this comment.
Probably could be better named, but this actually also indicates who provides the ordinal.
There was a problem hiding this comment.
Would something like is_offer_owner make more sense? Or do you have a better suggestion?
There was a problem hiding this comment.
ah gotcha, i understand now. yes, that name would work better for me i think. and then the corresponding comment could be updated as well. If you'd like I can make this change and include it in a PR to this branch, or feel free to do it yourself.
| /// The descriptor for the contract outcomes. | ||
| pub outcome_descriptor: OrdOutcomeDescriptor, | ||
| /// The original location of the ordinal within the UTXO set. | ||
| pub ordinal_sat_point: SatPoint, |
There was a problem hiding this comment.
Maybe this could be called ordinal_output_index as sat point sounds to me something like where the sat lives inside a single utxo
There was a problem hiding this comment.
SatPoint is the name used in the ord implementation: https://github.com/ordinals/ord/blob/master/src/sat_point.rs#L4
output_index seems like the index of the output within the transaction, but it's more than that since it also provides the offset for the actual satoshi.
There was a problem hiding this comment.
Ok, if that's how ORD does it, let's stick to the structure!
| let payout = if i % 2 == 0 { | ||
| Payout { | ||
| offer: TOTAL_COLLATERAL, | ||
| accept: 0, |
There was a problem hiding this comment.
This logic feels a bit fragile, here and down where creating the OrdEnumDescriptor to_offer_payouts. the i % 2 logic in both places takes some brainpower to follow and a bug could sneak it's way in. maybe there's some way to hard-code the outcomes in a clearer way.
There was a problem hiding this comment.
Yeah there are many ways to decide the payouts depending on the outcome. This one is just giving everything to one party for outcomes in odd position and the other party for outcomes in even positions. I thought it nice because it's not too simple while still not being too complicated IMHO (if we do e.g. the first half to one party and the other half to the other party it feels easier to miss some possible issues). But I don't have a strong opinion so let me know what you would prefer.
There was a problem hiding this comment.
OK, I'll review it more and think about whether I could like another design better. Maybe I'll propose something in a PR and we can discuss. Thanks for the feedback. Anyway, this isn't a blocker
| pub descriptor: EnumDescriptor, | ||
| /// The outcomes for which the ordinal is given to the offer party (one boolean for each | ||
| /// possible outcome). | ||
| pub to_offer_payouts: Vec<bool>, |
There was a problem hiding this comment.
Could we call this "to_offeror_payouts" ? to_offer makes it sound like it's mapping something to an Offer type or something, rather than the conditions when the ordinal goes to the offeror.
There was a problem hiding this comment.
offer is what is used consistently in the code base and the DLC specifications so I'd rather not have it different just here.
8519abd to
3de2153
Compare
Co-authored-by: Thibaut Le Guilly <thibaut@cryptogarage.co.jp>
Co-authored-by: Thibaut Le Guilly <thibaut@cryptogarage.co.jp>
3de2153 to
76179a4
Compare
2 participants
This PR adds the possibility to use an ordinal as part of the collateral of a contract. See
docs/Ordinal.mdfor more details.