OmniLayer / omnicore

OmniCore staging tree
http://www.omnilayer.org/
MIT License
758 stars 231 forks source link

Reject transactions involving BTC, if there is already one (?) #166

Open dexX7 opened 9 years ago

dexX7 commented 9 years ago

I'm currently going through the (traditional) DEx logic, and the logic for DEx payments made me wonder:

But wait, this means, if the seller has an offer for MSC and TMSC, then only the MSC part will be served. Let's check the specification:

Note: An address cannot create a new Sell Mastercoins for Bitcoins offer while that address has any active offer that accepts Bitcoins. Currently, this includes an active Sell Mastercoins for Bitcoins offer (one that has not been canceled or fully accepted and full payment received) and an active New Property Creation via Crowdsale with Variable number of Tokens that accepts Bitcoins.

As the commit I linked shows, this passage was updated, and previously it stated that there can not be more than one offer for the same currency.

Even though I created a test for exactly this, it slipped through, because I created the second offer for the same property. The updated regtest fails currently.

I don't consider it as bug, because the spec amendment was made in retrospective (in the context of crowdsales with BTC IIRC).

Nevertheless, handling this properly is desired, and I see two options, both requiring an extension of the currently used consensus rules in Omni Core:

  1. We reject attempts to create more than one DEx offer.
  2. We require that BTC payments explicitly specify what they are referring to.

I would really favor the second choice, which is much less restrictive, and more intuitive. While also a bit more complex, it would provide a massive improvement for BTC based crowdsales, and we're at a point where new transaction types or mechanisms can be created rather quickly, though I'm wondering, how this mechanism could look like exactly.

Anyway, I think we should address it in one way or another.

/CC: @marv-engine, @CraigSellars

zathras-crypto commented 9 years ago

I really like the idea of requiring that DEx payments specify what they are referring to. I much prefer the idea of thinking of a DEx payment as a regular payload/message like any other, the fact the transaction also carries BTC is a secondary, the primary is the payload telling the system what to do with said BTC.

Note that doesn't necessarily resolve the more than one DEx offer, because that happens at the accept stage. I'm pro having the accept specify the sell offer being accepted which I think is needed for more than one DEx sell. That also opens up "market making" (sells at a range of prices) from just one address (where as one of the early complaints was that to market make you had to spread over a range of addresses).

though I'm wondering, how this mechanism could look like exactly.

I actually think we should just keep this simple, and have a new "Pay BTC to recipient" transaction type in the spec. The payload would have a type, version and txid. That way we could send BTC to any recipient for payment of txid. That txid could refer to a DEx sell, a BTC crowdsale, or any future transaction that needs payment in BTC.