rsksmart / rskj

RSKj is a Java implementation of the Rootstock protocol.
https://rootstock.io
GNU Lesser General Public License v3.0
670 stars 268 forks source link

Bridge - btc fee payed by recipient improvements #21

Open aeidelman opened 7 years ago

aeidelman commented 7 years ago

See https://github.com/bitcoinj/bitcoinj/pull/1352

SendRequest.tx is modified even when Wallet.completeTx() throws ExceededMaxTransactionSize exception https://github.com/bitcoinj/bitcoinj/issues/1359

Wallet.completeTx() with P2SH multisig may not detect too big txs https://github.com/bitcoinj/bitcoinj/issues/1358 This is more likely to be reproduced if we implement the add/remove federators feature and have to send all the federation money to a new federation. Or when we group release txs Fee refactor: Sync fee calculation with Core 0.14 https://github.com/bitcoinj/bitcoinj/issues/1356 Understand why changeOutput.getValue() is added to fee? I can't see any effect of this if (changeOutput.getMinNonDustValue().isGreaterThan(changeOutput.getValue())) { // Never create dust outputs; if we would, just // add the dust to the fee. fee = fee.add(changeOutput.getValue()); When selecting coins to spend, don't select coins that are "dust to me" given the feePerKb we use: If to spend an UTXO we have to add an input whose size makes the fee increase bigger than the UTXO value, don't pick that UTXO.

A change output may not be dust for the network, but may be "dust to me". If adding a change output (not dust) makes the tx to pay an extra fee bigger than the change output, throw the change to fees (or make recipients pay the difference to take it out of dust if SendRequest.recipientsPayFees is true). Also, a P2SH output may require a big 10 of 15 multisig script sig, so the size of the script sig required to spend it may be taken into account. Create a boolean SendRequest.avoidDustToMe to implement this feature If change output is dust, split the cost of getting out of dust between all the recipients' outputs, not just the first output.

See https://github.com/bitcoinj/bitcoinj/pull/1352

See  https://github.com/bitcoinj/bitcoinj/pull/1352 

colltoaction commented 5 years ago

@pamgonzalez @pmprete @amendelzon @josedahlquist can anyone confirm this is still an issue?

amendelzon commented 5 years ago

Yes, we never tackled any of these.