bcoin-org/bcash

Can't createTX() with OP_RETURN output

Closed this issue · 17 comments

gtklocker commented

The output is considered dust so the function throws, however a tx with an OP_RETURN is still a valid tx.

dionyziz commented

Can you try increasing its value?

bucko13 commented

Yeah, note that even OP_RETURNs should need to pay a fee.

gtklocker commented

Is this right? I mean there definitely will be a fee (on the whole transaction) but I've never seen the value of specifically the OP_RETURN output be something other than 0. Example: https://live.blockcypher.com/btc/tx/88f89cecc445d4e62a5d5c2ccbf6e512348b18ff2b9be23473d62f9660629846/

Also, createrawtransaction on Bitcoin-ABC when passed a data key will also create a tx with zero value on the OP_RETURN output, fundrawtransaction or signrawtransaction do not affect it. And it does get relayed/confirmed just fine.

OrfeasLitos commented

It seems like each output needs to be above the dust limit (which in Bitcoin Core with default parameters for a simple P2PKH turns out to be 546 shatoshis): https://www.reddit.com/r/Bitcoin/comments/2unzen/what_is_bitcoins_dust_limit_precisely/ bitcoin/bitcoin@9022aa3

I couldn't find a definitive answer though.

gtklocker commented

Again, this is a confirmed Bitcoin transaction from 3 hours ago:

https://live.blockcypher.com/btc/tx/88f89cecc445d4e62a5d5c2ccbf6e512348b18ff2b9be23473d62f9660629846/
http://chainquery.com/bitcoin-api/getrawtransaction/88f89cecc445d4e62a5d5c2ccbf6e512348b18ff2b9be23473d62f9660629846/1

As you can see, the OP_RETURN output has 0 value:

...
			{
				"value": 0.00000000,
				"n": 0,
				"scriptPubKey": {
					"asm": "OP_RETURN 6f6d6e69000000000000001f0000000c59684c18",
					"hex": "6a146f6d6e69000000000000001f0000000c59684c18",
					"type": "nulldata"
				}
			},
...
gtklocker commented

@OrfeasLitos with regards to the bitcoin source you cited, it can easily be seen that there's special treatment for non-spendable outputs. https://github.com/morcos/bitcoin/blob/f4d00e63f7951469c840edede3f675d07249f62a/src/policy/policy.cpp

CAmount GetDustThreshold(const CTxOut& txout, const CFeeRate& dustRelayFeeIn)
{
    if (txout.scriptPubKey.IsUnspendable())
        return 0;
...
}

bool IsDust(const CTxOut& txout, const CFeeRate& dustRelayFeeIn)
{
    return (txout.nValue < GetDustThreshold(txout, dustRelayFeeIn));
}

This would mean that IsDust on a 0-value OP_RETURN should evaluate to 0 < 0 which in turn is false.

  • 👍2
OrfeasLitos commented

I think you found here the crucial spot. You can make a PR here if you have the time.

gtklocker commented

By the way, OP_RETURN outputs don't even go through the IsDust check: https://github.com/morcos/bitcoin/blob/f4d00e63f7951469c840edede3f675d07249f62a/src/policy/policy.cpp#L143-L144

The only limitation is, you can have only one such output.

gtklocker commented

@bucko13 any update on this? There's the exact same issue on bcoin, maybe we should open an issue there?

bucko13 commented

Sorry for the confusion. My point wasn't that the OP_RETURN output needs to have the fee since fees are basically paid by inputs, just the transaction as a whole that's creating it. I've personally sent OP_RETURNs with bcoin before (though with bcoin but this should be the same for both) so it should work and you're right that it shouldn't do a dust check, which I don't think it does. So my guess is that the problem is with the other output, the change, being dust.

This could be a bug with the MTX fund method picking a coin as an input that's too small for fee + change. More likely though there's something wrong with how the tx is being composed. Could you share a snippet of how you're creating the tx and what the JSON for the tx is?

P.S. if you're interested in seeing bcoin/bcash code that composes valid OP_RETURN outputs (including multiple outputs for 1 tx), there's a bpanel plugin that you can take a look at and play around with on your own node if you get bpanel setup: https://github.com/bpanel-org/publish-data

gtklocker commented

Here's a full minimal example which fails:

const bcash = require('bcash');
(async () => {
  const node = new bcash.SPVNode({});
  const {wdb} = node.use(bcash.wallet.plugin);
  await node.open();
  await node.connect();

  const wallet = wdb.primary;
  await wallet.send({
    outputs: [bcash.Script.fromNulldata(Buffer.from('foo'))]
  });
})().catch((err) => {
  console.error(err);
  process.exit(1);
});

Output:

Error: Output is dust.
    at Wallet.createTX (/home/vagrant/bch-nipopow-prover/node_modules/bcash/lib/wallet/wallet.js:1219:15)
    at Wallet._send (/home/vagrant/bch-nipopow-prover/node_modules/bcash/lib/wallet/wallet.js:1284:28)
    at Wallet.send (/home/vagrant/bch-nipopow-prover/node_modules/bcash/lib/wallet/wallet.js:1269:25)
    at process._tickCallback (internal/process/next_tick.js:68:7)
gtklocker commented

With regards to your hypothesis, as I stated earlier, the problem is in createTX (link to problematic code).

As you can see all outputs are tested for dustedness unconditionally, so this also checks the OP_RETURN which fails. At this point fund isn't called yet. This can also be seen from the stack trace above.

gtklocker commented

No. I've ensured the wallet has enough balance, and can serve the transaction just fine. Creating the tx manually, funding and sending it does work.

OrfeasLitos commented

Could you also provide the minimal testcase that works as well? It will make comparison easier.

bucko13 commented

outputs should be an array of output objects. In your example script, you're just passing a Script class as an item on the array which send won't know how to interpret. Luckily the Output class in bcoin/bcash has a fromScript method you can use to convert your OP_RETURN script, as seen in the bpanel plugin mentioned above (link to relevant code here).

gtklocker commented

You're right.