namecoin / namecoin-core

Namecoin full node + wallet based on the current Bitcoin Core codebase.
https://www.namecoin.org/
MIT License
455 stars 146 forks source link

RPC API for atomic name trades #360

Open yanmaani opened 4 years ago

yanmaani commented 4 years ago

Atomic name trades are desired. The question is how to develop the API for them.

The initial reflex might be to make two commands as such: name_makeoffer "id/example" 1000 direction=buy locktime=520000 name_takeoffer <PSBT>

(The reason to include locktime is to facilitate automated non-interactive Dutch auctions.)

However, this is a massive footgun. A system where you paste arbitrary hex blobs and sign them will inevitably lead to loss of funds.

Thus, any command where you accept an offer ought to require you to specify the terms thereof in a human-readable format as well. So it would be safer with something like:

name_takeoffer "id/example" 1000 direction=buy <PSBT>

If you do that, they begin to look very similar. Is it then prudent to merge them into one RPC?

Also, having a parameter that takes one of two strings is sort of awkward. Making it a boolean is also awkward, and it would not be good if you could mess them up by accident.

In light of these considerations, anyway, the following API seems to me reasonable:

name_buy "id/example" amount {"locktime": N, "offer": PSBT} name_sell "id/example" amount {"locktime": N, "offer": PSBT}

Practical uses would look like:

name_buy "id/example" 100.01 {"locktime": 1596412800} - if "id/example" is registered and held by someone else, return a signed PSBT offer to buy that enters into effect on Mon Aug 3 00:00:00 UTC 2020, otherwise throw an error name_buy "id/example" 100.01 {"offer": <big hex string>} - if that PSBT is correct with regard to specified name, price, and direction, accept it, otherwise throw an error

However, I not sure about what is good UX here. It's very important that the API be good, because it will be a lot harder to change later.

yanmaani commented 4 years ago

It might also be good to have some option (true by default) to warn on nonstandard names, so nobody buys "example.bit," "D/example," or "d/példány" (as opposed to "d/example" or "d/xn--pldny-zqa7c"). On the other hand, there has to be some limit to the hand-holding. If you try to buy "d/examp1e," that clearly falls under caveat emptor.

The GUI should obviously have such a feature, and then it might make sense to expose it as to the RPC API as well. Psuedocode for validation I imagine to look something like:

if (!name.contains("/") return true;
prefix = name.split("/")[0];
tail = name.split("/")[1:].join("/");
switch (prefix) {
    case "d":
        return !is_valid_dns_label(tail);
    case "id":
        ???;
    default:
        return true;
}

Thoughts?

JeremyRand commented 4 years ago

I think we should probably handle this in layers. Something like this:

walletcreatenameofferpsbt would accept the following params:

  1. name, the name identifier.
  2. amount, the amount of the trade in NMC.
  3. direction, either "buy" or "sell".
  4. interactive, boolean.
  5. locktime/options/bip32derivs, same as walletcreatefundedpsbt.

Implementation would look like this:

  1. If direction is sell:
    1. inputs = array of length 1, set to the name input returned by name_list(name).
    2. outputs = array of length 0.
    3. tx = walletcreatefundedpsbt(inputs, outputs, locktime, options, bip32derivs).
    4. At this point, tx contains the name input and a currency change output.
    5. outputs = outputs of tx.
    6. At this point, outputs[0]'s amount hasn't yet added the trade amount.
    7. outputs[0][amount] += amount
    8. tx = createpsbt(inputs, outputs, locktime, options[replaceable])
    9. If interactive:
      1. tx = walletprocesspsbt(tx, sign=false, bip32derivs=bip32derivs)
    10. If not interactive:
      1. tx = walletprocesspsbt(tx, sighashtype=SINGLE|ANYONECANPAY, bip32derivs=bip32derivs)
  2. If direction is buy:
    1. inputs = array of length 1, set to a currency input in the wallet of at least amount.
    2. outputs = array of length 0.
    3. Same step as sell.
    4. At this point, tx contains the currency input and a currency change output (which we will convert to a name output).
    5. Same step as sell.
    6. At this point, outputs[0]'s amount hasn't yet subtracted the trade amount or added the name op.
    7. outputs[0][amount] -= amount; outputs[0][name_op] = name_update for name.
    8. Same step as sell.
    9. Same step as sell.
    10. Same step as sell.

decodepsbt would add a name_trade field, which would be populated as follows:

  1. If len(inputs) != 1 orlen(outputs) != 1` return null.
  2. If inputs[0][name_op] == null andoutputs[0][name_op] == null` return null.
  3. If inputs[0][name_op] != null andoutputs[0][name_op] != null` return null.
  4. If inputs[0][name_op] != null then direction = sell, name = inputs[0][name_op][name], amount = outputs[0][amount_display] - inputs[0][amount_display].
  5. If outputs[0][name_op] != null then direction = buy, name = outputs[0][name_op][name], amount = inputs[0][amount_display] - outputs[0][amount_display].
  6. If inputs[0][sighash] ==SINGLE|ANYONECANPAYtheninteractive = true; elseinteractive = false`.

This can be used to verify that an offer is what it purports to be.

Finally, walletacceptnameofferpsbt would accept the following params:

  1. offer, the name offer.

And would be implemented like this:

  1. name_trade = decodepsbt(offer)[name_trade]
  2. inverted_offer = walletcreatenameofferpsbt(name=name_trade[name], amount=name_trade[amount], direction=invert(name_trade[direction]), interactive=true, ...)
  3. tx = joinpsbts(offer, inverted_offer)
  4. tx = walletprocesspsbt(tx, ...)
  5. return tx

Note that the above workflow is pretty awkward because we're recreating the PSBT several times. It probably makes more sense to replace a lot of these RPC calls with lower-level API calls that directly manipulate the fields inside the PSBT, so we can e.g. append inputs/outputs to a PSBT without losing the existing signatures. It looks like psbt.h has a lot of relevant stuff.

Anyway, that's my first pass at this.

domob1812 commented 4 years ago

Is that actually something we want in Namecoin Core? You can already do all that with the raw transaction API. I think it would be better to handle using that API in a more user-friendly way in an application built on top of it, like @phelixnmc's antpy.

JeremyRand commented 4 years ago

Is that actually something we want in Namecoin Core? You can already do all that with the raw transaction API. I think it would be better to handle using that API in a more user-friendly way in an application built on top of it, like @phelixnmc's antpy.

@domob1812 Yes, it's something we want in Namecoin Core. Or, at least, NLnet wants atomic trades to be accessible from the Namecoin-Qt GUI for UX purposes, such as:

  1. Right-click a name in Manage Names, an option appears for "Sell this name", which will produce a PSBT you can save.
  2. Try to register a name that already exists, an option appears for "Buy this name", which will produce a PSBT you can save.

While I suppose it is theoretically possible to have Namecoin-Qt outsource this to an external program (which then outsources the transaction construction back to the Namecoin Core RPC PSBT API), this is clunky and it's not clear to me what the benefit would be. (I think the security benefits from keeping this in a separate process are dubious.)

Also it should be noted that the raw transaction API is not well-suited for this, because the transactions it produces do not follow a specification, and therefore other wallets (e.g. Electrum-NMC) cannot read or create them. PSBT is the successor to the raw transaction API, and it does have a spec (which Electrum-NMC supports as of v4.0.0).

Anyway, as far as I can tell, there are major shortcomings in Bitcoin Core's PSBT RPC API, in that it does not appear to be designed with the intent of adding inputs or outputs after some signatures have already been added (which is a requirement for non-interactive atomic name trades). There's nothing in Bitcoin that should prevent this (it's what all SIGHASH flags other than SIGHASH_ALL are designed for), and AFAICT psbt.h does allow this, but the RPC API does not seem to expose this ability. @domob1812 , does this match your understanding? My preference is to ask upstream Bitcoin Core if they would be willing to accept a PR that adds this ability, rather than improvise something name-specific ourselves, since presumably there exist non-Namecoin smart contract constructions that would benefit from a standard RPC API for this.

I'm fine with asking upstream Bitcoin Core about this, if we can form a consensus that this would be the preferable approach. Thoughts?

domob1812 commented 4 years ago

Yes indeed, the PSBT API is kind of lacking. We've had an external developer for Xaya go through using the raw and PSBT RPC APIs to do something like this, and it was indeed non-trivial to come up with a flow that works. I'm definitely in favour of trying to get proper support of all the basic operations into upstream Bitcoin, so that PSBTs can be used instead of raw transactions for the full atomic-trading workflow. If upstream Bitcoin signals interest but the devs have no time to do it for us in a timely manner, I guess we could use some NLnet and/or Handshake funds to do it upstream (I'd even be available to do it myself if Autonomous Worlds Ltd gets paid cost-price for my time).

Once this is done, we can revisit what else we need in Namecoin Core that is name-specific. My feeling is that after the PSBT APIs are adjusted accordingly in a non-name-specific way, then it will be almost trivial to build the trading UX in the Qt UI, so that we won't really need specific RPCs for it anymore (and instead the UI code itself can just do it via the PSBT API).

JeremyRand commented 4 years ago

Regarding PSBT API improvements, here's one that we can do on our end: #361

JeremyRand commented 2 years ago

@yanmaani What's the status on this; is anything in Bitcoin Core blocking work on this?

yanmaani commented 2 years ago

Not that I can remember, no. I have some code, I think the issue is either that I'm not done with the tests or that I haven't figured out a clean way to lock the coins. I'll look at it once I get back to it, but right now there's other stuff above it on the to-do list.