Send OPENs to nulldata address

[pinheadmz]

Closes #481
Chips away at #478
Requires network deployment (especially miners) of #498 ✅

Motivation is partly explained in #481 but here are the pros and cons:

PROs

• reduce size of OPEN covenants by 18 bytes
• OPEN covenants do not bloat the UTXO set with 0-value outputs (technically spendable but why would you?)
• OPEN covenants do not bloat the wallet's UTXO set, which adds to the burden of the coin selector
• Prevents an annoyance where an attacker can send an OPEN to someone else's wallet, forcing that wallet to watch and collect all bids for that auction, whether or not the unwilling recipient cares about the name (also adds a 0-value UTXO to the victim's wallet, because OPENs aren't checked for dust) <- This will require a second patch in txdb if we decide it is a big enough issue.

CONs

• We must wait until a majority of the relay nodes and miners have updated with [POLICY] address: fix isUnknown() to allow nullData and version 0 addresses #498 which fixes a bug in relay policy.
• Because there is no address associated with OPEN, applications that used to use the address to detect when a wallet has sent an OPEN must be refactored to instead check the inputs to the TX ("Did my wallet send this tx? Then OK, tell the user they have OPENed an auction" -- as opposed to "This OPEN is being sent to my own address -- example in Bob Wallet")

Review

commit 4b75f8a address: don't restrict data length to 20 or 32

• I'm not sure if we ever needed this check in the first place, opened an issue upstream: Address getHash requires data length of 20 or 32 only bcoin-org/bcoin#989

commit 0301f47 wallet: do not remove wallet-name-map when disconnecting a block

• At the beginning of a rescan, a lot of wallet state is thrown out (not transactions themselves, but whether or not they are confirmed and in what block). We also clear out the name map, which is a simple structure that stores which wallet IDs are "watching" a name. I do not think we need to reset this, and removed it here.

commit 4075ae8 wallet: send OPENs to unspendable nullData address

• this is the fix, comments should explain the patch. Luckily we have importName() now! Wallet: enable importing names and "watch" for auction activity without bidding #408

commit 8edaea6 test: OPEN outputs no longer inserted as coins into txdb

• notice how certain tests had to be adjusted because the OPEN output is no longer stored as a coin in the wallet

commit c193247 pkg: update CHANGELOG

• hopefully this explains the change well enough to developers using hsd in their own wallet applications

[pinheadmz]

@turbomaze

[coveralls]

Pull Request Test Coverage Report for Build 209047434

• 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
• 12 unchanged lines in 4 files lost coverage.
• Overall coverage decreased (-0.01%) to 58.982%

---

Files with Coverage Reduction	New Missed Lines	%
lib/mining/miner.js	1	64.12%
lib/wallet/rpc.js	1	21.02%
lib/wallet/walletdb.js	2	76.05%
lib/wallet/txdb.js	8	82.51%

Totals
Change from base Build 204174244:	-0.01%
Covered Lines:	19279
Relevant Lines:	30402

---

💛 - Coveralls

[turbomaze]

Thanks zippy! This is a great change

[pinheadmz]

Back when I wrote this PR I tried to test it so I could add an example to the description here. I didn't realize in advance that nulldata addresses were accidentally classified as nonstandard and would be rejected by all network nodes and not relayed. I actually set my own node to accept non standard transactions to complete the test. That transaction (an OPEN for the name nulldata) was finally confirmed in a block last week. The auction opened normally, received one bid and is currently in the reveal phase.

Because this was (I think) the first nulldata address on the chain, HNScan.com actually crashed on this block because of a an error in its backend when parsing such an address (unrelated to hsd or this PR).

Anyway - the test basically worked. We can open auctions with nulldata addresses and apparently there are enough v2.2 nodes on the network (including miners) for such transactions to get confirmed. I hesitate to merge this PR too soon though because relay may still be quite choppy. Here's the peer version breakdown from easyhandshake today:

Map {
  '/hnsd:0.0.0/' => 3,
  '/hsd:2.0.1/' => 2,
  '/hsd:2.0.2/' => 21,
  '/hsd:2.0.3/' => 2,
  '/hsd:2.1.1/' => 3,
  '/hsd:2.1.2/' => 19,
  '/hsd:2.1.3/' => 9,
  '/hsd:2.1.5/' => 99,
  '/hsd:2.2.0/' => 42
}

and the TX that broke everything :-)

{
  "hash": "12f0b0cd96c1ffbcdda145fb3a3f8580b756805ba9c47836e5b018f857096dd7",
  "witnessHash": "1b6dc72c33face502ea341092b81d1db7bb46c739eabce81ab62c824cde08e73",
  "fee": 16900,
  "rate": 100000,
  "mtime": 1600192815,
  "height": 33058,
  "block": "000000000000000186256e2aa609ec3782076eb784e92d3264c9b22579690e99",
  "time": 1600178873,
  "index": 1,
  "version": 0,
  "inputs": [
    {
      "prevout": {
        "hash": "229fec21746714b3ec2c3f28ac8d1843f3f864f4bf6b67028e8c80f3c883178a",
        "index": 1
      },
      "witness": [
        "ab8e2e20da0fe6d7ef0e8ebd64c1201f6f3691da643e226fc88e7b7fadcfd2234eabb53f0a34fd30f82d0429ef7c57e85d60096b7e023ac7df6a068465f048fe01",
        "02d61a5afbd7057d2db6128594c4d06bfae6e7d85ad1aca5ea1aa7e46d1762989f"
      ],
      "sequence": 4294967295,
      "coin": {
        "version": 0,
        "height": 11685,
        "value": 6281800,
        "address": "hs1qxmsxlz4465uupqdtxrzq8y6p24633ht5r97smu",
        "covenant": {
          "type": 0,
          "action": "NONE",
          "items": []
        },
        "coinbase": false
      }
    }
  ],
  "outputs": [
    {
      "value": 0,
      "address": "hs1lqqqqhuxwgy",
      "covenant": {
        "type": 2,
        "action": "OPEN",
        "items": [
          "109e1992c74d0fcabbc6f3774268f967022c4884fad33b6853341792183329b9",
          "00000000",
          "6e756c6c64617461"
        ]
      }
    },
    {
      "value": 6264900,
      "address": "hs1qlje6h3zzy8syqu69p9jx9lh5hfqngmse079uf4",
      "covenant": {
        "type": 0,
        "action": "NONE",
        "items": []
      }
    }
  ],
  "locktime": 0,
  "hex": "0000000001229fec21746714b3ec2c3f28ac8d1843f3f864f4bf6b67028e8c80f3c883178a01000000ffffffff0200000000000000001f020000020320109e1992c74d0fcabbc6f3774268f967022c4884fad33b6853341792183329b90400000000086e756c6c6461746144985f00000000000014fcb3abc44221e0407345096462fef4ba41346e190000000000000241ab8e2e20da0fe6d7ef0e8ebd64c1201f6f3691da643e226fc88e7b7fadcfd2234eabb53f0a34fd30f82d0429ef7c57e85d60096b7e023ac7df6a068465f048fe012102d61a5afbd7057d2db6128594c4d06bfae6e7d85ad1aca5ea1aa7e46d1762989f",
  "confirmations": 855
}

[nodech]

Maybe I missed it,

but does not this need to change rescan logic as well, so wallets can recover the OPENs they have sent?

[pinheadmz]

importName() will add the name to the wallet's name map, which means the wallet will "track" all auction activity like bids etc even if the user did not bid themselves (this is weird, but its how the wallet works already). This will persist for rescans as well, especially because of this change: 0301f47

Since the wallet sent the open, it should recover the TX normally because its own keys were used for the inputs and change.

OPENs are weird things - they are just a marker for the chain and wallet (this is why they have 0 value), announcing to the network a namehash and its preimage. JJ told me OPENs weren't even added to the system until late in development because they didn't seem necessary, but definitely made wallet logic a lot easier.

We have a few rescan tests now in hsd, I'll rebase this PR and we can double check

[nodech]

sure,

I was wondering about importing wallet into fresh wallet instance.
I can test after rebase.

[pinheadmz]

I was wondering about importing wallet into fresh wallet instance.

Yes! this needs to be patched. Its' going to be in txdb.insert() or txdb.confirm() I think

[pinheadmz]

Closing this for now, I think the approach is a bit flawed. First of all, the bloat is not that bad. An OPEN UTXO is at most only about 130 bytes, meaning this year with over a million names on chain we're only "wasting" 130 MB. Second, sending to nulldata address just to make an output unspendable seems a bit hacky. What we should do is just... make 0-value OPENs unspendable! Whether or not that ever becomes a consensus rule, it can certainly be factored in to the wallet so we don't add these coins to the txdb.

What makes the most sense is to refactor the coinselector in the wallet, and refactor the layout of txdb so coins can be pulled out by value (instead of randomly by hash and then filtered by value and spent-ness)

[Falci]

make 0-value OPENs unspendable

What would happen to the "0-value OPENs" already spent, when a new node tries to sync?

[pinheadmz]

make 0-value OPENs unspendable

What would happen to the "0-value OPENs" already spent, when a new node tries to sync?

This is why soft forks have activation height!