The current fee calculation calcFee() does not take into consideration the OP_RETUN output, if present.
This Diff make sure it does
Details
Details
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- fix-fee-calculation (branched from master)
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 18283 Build 36371: Build Diff cashtab-tests Build 36370: arc lint + arc unit
Event Timeline
Comment Actions
This approach seems over complicated. For example, I don't think we will run into the case of a tx with 65,535 outputs, much less 4294967295 outputs (node won't broadcast a tx of > 100kb).
- The easiest way to handle this would be to add an output, since an op_return tx should have an additional ouput (instead of the usual two, one for sent and one for change, you will have 3). p2pkhOutputNumber = 2 should be p2pkhOutputNumber = 3 for op_return txs
- We can get more specific by getting the bytecount of this output from the length of the msg. It's not necessary to consider all of the minutia around bitcoin transactions to get the size of the OP_RETURN tx. Knowing its length is enough. For example this tx has the msg, "first":
Here's the output:
6a0400746162054669727374
6a - this is an OP_RETURN tx
04 - here comes a 4 byte prefix (hardcoded in Cashtab)
00746162 - the prefix
05 - here comes a 5-byte msg
4669727374 - the 5-byte message, "First" (http://www.unit-conversion.info/texttools/hexadecimal/)
So, we can assume that a n-byte message will be (1 + 1 + 4 + 1 + n) bytes, in this case 12 bytes total for a 5 byte message.
I think best approach here is
- See how adding one more p2pkOutputNumber impacts the fee calculation. Does it just add 223 bytes to byteCount?
- Use the above simplified formula (byteCount of an OP_RETURN output = 7 + msgLength) to get byte count of an OP_RETURN output based on message length. Cashtab always sends a message with a 4-byte prefix, so it's okay to make this assumption.
Comment Actions
I was trying to be precise since we are dealing with 'money'.
One or two satoshis may not seem much now but may worth much more in the future.
Beside, the code is pretty simple once we are clear about the structure of a tx.
However, if the goal is the simplify, I suggest we calculate the opreturn fee as follow:
- 8 bytes for the length of Output Value
- 1 byte for the length of Locking Script
- n bytes for the length of OP_RETURN Script
- 2 bytes in case there more than 252 outputs (we will be paying extra 2 bytes in most cases)
So the fee = satoshisPerByte * ( byteCounts + 11 + opRetunLength)
This would be more simple and traightforward comparing to adding an extra p2pkh output to account for the OP_RETURN.
Comment Actions
Okay cool.
- Remove the edge cases for varint due to parseInt('0xffff') and parseInt('0xffffffff'). We don't need to account for transactions with parseInt('0xffff') or more outputs (although pretty interesting fun fact about bitcoin txs...TIL). In practice, transactions can't have more than 2,500 or so outputs due to the 100kb restriction.
- Bump opReturnLength by 7, as this will be the delta between what the user (and the sendXec function) submit, and what is actually put on the blockchain
(7 =
+1 for OP_RETURN marker 6a
+1 for 04 for prefix length
+4 for 4-byte prefix
+1 for OP_RETURN msg length varint
)
- Add unit tests based on real txs, i.e. ensure that determined bytecount is equivalent to something like
'0200000001408883ae90ba3957a9f1e60778b4606133f75c8605050ce1c8678051cdeb76d3020000006a47304402200b5ec43a061433270fc092c496f22b33ad2fb2718b49203fb5fea59430e025a00220084b015f3b8d4015ac4b3817f62efb328427e6de1ad55441c06b8a5e9c80db8c412103771805b54969a9bea4e3eb14a82851c67592156ddb5e52d3d53677d14a40fba6ffffffff0300000000000000000c6a040074616205466972737410270000000000001976a9144e532257c01b310b3b5c1fd947c79a72addf852388ac2b237d01000000001976a91495e79f51d4260bc0dc3ba7fb77c7be92d0fbdd1d88ac00000000'.length / 2
Add some actual raw txs of OP_RETURN to a mock file for the unit test.
You can get a raw tx from a txid from this endpoint. Here are some example txs I made for testing. Can also make your own with longer / shorter msgs, no msgs, eToken txs.
https://rest.kingbch.com/v4/rawtransactions/getRawTransaction/55bc0e376b923e923e3b114d8b81423837835da4a5e782353fc9c087033a9bfb
https://rest.kingbch.com/v4/rawtransactions/getRawTransaction/04e13833b7de3656ba436be8b3f2286399a03053451b3f753c8928cd4972aaea
https://rest.kingbch.com/v4/rawtransactions/getRawTransaction/759fd5de82d3b4744be54a4cd5428d63b349822268a37c9c53279a9d86d2020c
https://rest.kingbch.com/v4/rawtransactions/getRawTransaction/965052b661e086f2d2d3d0647c86e57e28bf5e72d6e6ab3f63b0744b442b1ae9
Here's a good tool for decoding raw txs: https://live.blockcypher.com/btc/decodetx/
From this, you can get the actual tx size of txs with varying numbers of inputs and outputs (and varying OP_RETURN length).
Comment Actions
Responding to review feedback- made relevant changes, commented out broken tests per discussion with Joey to help with next steps.