Rune buy offers

[joshdoman]

TLDR

This PR implements issue #4183 and updates the documentation in wallet.md. Users may create and accept rune buy offers for the entire balance of any UTXO, as long as it holds a single rune.

Details

• This PR only supports full balance buy offers on a single UTXO with a single rune balance. Multi-utxo offers, sub-balance offers, and offers on a UTXO holding multiple runes are not supported, as they types of offers would require a runestone.

• An error is thrown if the purchased UTXO does not match --utxo, if present (this applies to both inscription and rune buy offers)

• clap is used to throw an error if neither --inscription nor --rune is present, if both are present, or if --rune is present and --utxo is not.

Documentation

Creating an Inscription or Runes Buy Offer

Bid AMOUNT on the inscription INSCRIPTION_ID using:

ord wallet offer create --inscription <INSCRIPTION_ID> --fee-rate <FEE_RATE> --amount <AMOUNT>

Bid AMOUNT on the rune balance DECIMAL:RUNE at UTXO using:

ord wallet offer create --rune <DECIMAL:RUNE> --fee-rate <FEE_RATE> --amount <AMOUNT> --utxo <UTXO>

Accepting an Inscription or Runes Buy Offer

Accept the offer to buy the inscription INSCRIPTION_ID for AMOUNT in PSBT using:

ord wallet offer accept --inscription <INSCRIPTION_ID> --amount <AMOUNT> --psbt <PSBT>

Accept the offer to buy the rune balance DECIMAL:RUNE in PSBT using:

ord wallet offer accept --rune <DECIMAL:RUNE> --amount <AMOUNT> --psbt <PSBT>

[casey]

Nice, thank you for opening this!

Alright, initial thoughts, just on the implementation. I haven't looked at the tests at all, or really how the implementation works, just what it's doing.

• This PR should be scoped down so that a buy offer never creates a runestone. This means you can only buy an entire UTXO worth of runes. The UTXO must hold only a single rune, and you must be buying the entire UTXO worth. This is still a giant PR, so anything that makes it simpler will make it faster to get in, and this seems like something that could make it simpler.
• I'm not sure this should have two different code paths for buying runes vs buying inscriptions. There is a ton of duplication of the checks that the two paths makes. (E.g., checking for "no inscriptions" vs checking for "the one inscription that we want to buy", these could be a single check, which checks that the inscriptions in the utxo are equal to the inscriptions we want to buy, which could be an empty list.) So making it all a single code path feels like it might be simpler, and avoid the need for different tests for each code path. This also reduces the amount of code movement in the PR. Moving code shows up as lines added and removed in the diff, and makes review harder, since you've got to cross-reference the deleted code to make sure it's the same as the added code. Similarly, big refactors which move a lot of code are often simpler to do as separate PRs.
• Definitely only one UTXO at a time, this makes it much simpler.
• I think --postage can be added as a follow-up PR.
• A note on flag handling: Any flags which conflict or flags which require each other should be handled declaratively by clap. These produce good error messages up-front, without ever getting past argument parsing. You can then use assert!() or unreachable!() in the actual function body, and not worry about providing a good error message, since that will be done by clap, and you're just doing the assert!() or unreachable!() as a form of documentation, or to handle unreachable match branches.

A meta note: Often times before I start writing code, I have no idea what the right approach is, and it's in writing the code that I figure out what the right approach is. Often, this winds up being because as I write the code, as I'm writing it, I realize, for example, that one approach will require duplicating a bunch of code, and the other won't. So often times it's really hard for me to give good guidance up-front to contributors on how to approach a feature, because my own approach to writing the feature would be to just start writing it, and then see what looks good and works and what doesn't, and then change my approach from there. This also means that, since you've actually started getting your hands on the code, you might now have a better idea of what works vs what doesn't.

[joshdoman]

Thank you for the detailed feedback!

This PR should be scoped down so that a buy offer never creates a runestone.

Makes sense. I'll reduce the scope and leave runestone-related functionality for a future PR.

I'm not sure this should have two different code paths for buying runes vs buying inscriptions.

I think I'll take a stab at tearing this down and restarting with this in mind, so that everything runs in a single function. I worry that it may make the logic harder to follow and more difficult to update, but I won't know until I try. Your point about reducing the amount of code movement for sake of review-ability is well-taken.

Definitely only one UTXO at a time, this makes it much simpler.

👍

I think --postage can be added as a follow-up PR.

👍

Any flags which conflict or flags which require each other should be handled declaratively by clap.

I didn't know this was a feature of clap - very cool!

[joshdoman]

@casey I've stripped out the extra functionality like you suggested and refactored the implementation to properly use clap and keep everything in a single function, to minimize code movement.

Hopefully this will be easier to review!