Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OPEN covenants have 0 value #481

Open
pinheadmz opened this issue Jul 25, 2020 · 4 comments
Open

OPEN covenants have 0 value #481

pinheadmz opened this issue Jul 25, 2020 · 4 comments

Comments

@pinheadmz
Copy link
Member

pinheadmz commented Jul 25, 2020

As of July 5, block height 22517, the Handshake UTXO set included 152,153 OPEN covenants all with an output value of 0.

Here's a UTXO from one of the very first auctions ever (block 4032) that will never be purged from the utxo set:

{
  "version": 0,
  "height": 4032,
  "value": 0,
  "address": "hs1qflkm3n802zfzt0j9337d9hzxk4khctzj26t7sc",
  "covenant": {
    "type": 2,
    "action": "OPEN",
    "items": [
      "a8b64e7d7695f94eed27ec69ce9e02ed3f92784f38a9619abc25006a0a1d5571",
      "00000000",
      "656e64616e676572"
    ]
  },
  "coinbase": false,
  "hash": "1eb102b67b33463b4e63baac897cca6bbeef45a4ced538a77d1d35ab9feb93af",
  "index": 0
}

This is clearly a huge source of bloat. It consumes full node resources, and causes issues in the wallet by loading the txdb with coins it will never spend (#478 -- although there are several other improvements we need to make to fix that issue completely).

So, let's brainstorm some fixes.

One idea is to make OPEN outputs officially unspendable. REVOKE outputs are already unspendable and so are outputs to "nulldata" addresses (Handshake's segwit-only version of OP_RETURN). This would have to be enforced by a soft fork, and then OPENs would no longer be added to the utxo set. I did a quick grep of the wallet module for "spendable" and didn't find anything -- I hope the wallet knows which of its coins are unspendable! This actually might be another issue and we should make sure the wallet does not try to spend REVOKE outputs, and also prevent a wallet from thinking it can spend from a null address even if that address has been manually imported.

Another solution is to change the wallet behavior to use the OPEN covenant as a change output. This will reduce blockchain bloat and actually make these coins useful to the wallet. A typical open TX would then just have one input (any value) and one output (the OPEN) with the miner fee subtracted. This brings up kind of an interesting sub-issue though: should OPEN outputs in this scheme pay to a receive address or a change address (see BIP44)? Change address makes some sense, but then (for example) Bob Wallet would need to be updated to look for this, because otherwise it ignores change outputs and the user wouldn't see that a tx was an OPEN. Similarly, if we use receive addresses, Bob Wallet will need an update to recognize the tx as a self-send (I do not think it does this yet) otherwise the portfolio screen will appear to have an "incoming" OPEN tx that adds value to the wallet.

I think the latter change is best, but seems like it will require an update to Bob as well as informing users in general.

and finally -- is it insane to try and purge 0-value OPENs from the utxo set? That would also require a soft fork, and then also a utxo set database migration, which sounds scary as hell.

@turbomaze
Copy link
Contributor

OPENs as change is interesting but becomes complicated when you consider TXs with multiple OPEN outputs (more efficient due to batching). I like the idea of making OPENs unspendable, which perhaps is just a wallet-level change of paying into some black hole address. I can't think of any reasons someone would care about an OPEN output's address.

I would also want to implement the same changes for 0-value REDEEMs, but in that case, I'd also be satisfied if the wallet just neglected to generate REDEEMs for 0-value bids.

@pinheadmz
Copy link
Member Author

Oooh, here's a thought: Send all OPEN's to a nulldata address!

A nulldata address is the segwit-only version of OP_RETURN. It uses witness program version 31 (which is otherwise invalid) followed by at least 2 bytes (instead of a public key hash)

hsd.Address.fromNulldata(Buffer.alloc(2))
<Address: version=31 str=hs1lqqqqhuxwgy>

patch to hsd would be this, I think:

diff --git a/lib/wallet/wallet.js b/lib/wallet/wallet.js
index aae27623..a77f97a4 100644
--- a/lib/wallet/wallet.js
+++ b/lib/wallet/wallet.js
@@ -1561,7 +1561,7 @@ class Wallet extends EventEmitter {
     if (start !== 0 && start !== height)
       throw new Error('Name is already opening.');
 
-    const addr = await this.receiveAddress(acct);
+    const addr = Address.fromNulldata(Buffer.alloc(2));
 
     const output = new Output();
     output.address = addr;

These addresses are not included in the UTXO set and won't be stored as coins in the wallet (because that address does not belong to the wallet, and besides its unspendable)

The drawbacks to this are that the wallet will not "watch" the auction for future bids automatically, but we could use the same mechanism implemented in importname to replace this feature.

Agreed on the 0-value REDEEMs, that seems easy to add as well. Downside to that is wallets like Bob that will annoy the user if bids aren't redeemed.

@turbomaze
Copy link
Contributor

Yeah seems like a good solution!

@pinheadmz
Copy link
Member Author

As part of this issue, we may want to consider a soft fork that makes 0-value OPENs unspendable, and then migrate the UTXO set database to clear out the hundreds of thousands of unspendable coins. There is precedent for this in Bitcoin Core either when they upgraded to 0.8 or to 0.15 I can't remember, but there was a database migration that just didn't copy over the provably-unspendable coins to the new data set.

We would need to take the extra step of making these outputs unspendable first before actually pruning the coins.

@pinheadmz pinheadmz removed this from the v2.5.0 milestone Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants