-
Notifications
You must be signed in to change notification settings - Fork 42
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
Delay with confirming transaction due to insufficient fee #71
Comments
Started looking at this a few minutes ago. It's going to take me a bit to get re-acquainted with the code. I believe the code that will need to be changed is in Wallet.java in the core of litecoinj. It may be worthwhile to attempt to create a unit test for this scenario and code to that. |
Also I found this in litecoinj: public static final BigInteger MIN_NONDUST_OUTPUT = BigInteger.valueOf(5460); Perhaps tweaking that to 0.001LTC would allow the current logic written for Bitcoin to work. |
Probably not. This is the IsDust() limit, which is disabled in Litecoin in favor of the per-output fee penalty for all outputs smaller than 0.001 LTC. The alleged problem here is change outputs smaller than 0.001 are being created when change that small should instead be thrown into the fee. I had thought we already fixed this though ... |
Oh right - I thought we'd fixed this as well. I'll continue searching. |
OK the logic to remove an output due to dust penalty is here in SendCoinsFragment of bitcoin-wallet: // We have enough money to complete the transaction
// Now that the transaction is complete, adjust the outputs so change goes to fee
// if < 0.001 LTC. Otherwise, we would need to add a penalty for the output that is
// larger than the output itself, so it costs LESS to just throw it into the fee.
for(TransactionOutput o: baseSendRequest.tx.getOutputs()) {
if(o.isMine(wallet) &&
o.getValue().compareTo(Constants.CENT.divide(new BigInteger("10"))) < 0) {
Log.i(TAG, "Found a small change output of " + o.getValue() + "; putting in fee.");
// Removing the output means the value will go to fee
baseSendRequest.tx.removeOutput(o);
try {
baseSendRequest.setCompleted(false);
wallet.completeTx(baseSendRequest);
} catch (InsufficientMoneyException e) {
// This should never happen because we're only changing where outputs go
Log.e(TAG, "UNEXPECTED ERROR: InsufficientMoneyException when redirecting outputs!");
Toast
.makeText(getActivity(), "Unexpected Error: Insufficient Funds when moving change to fee. " +
"Please report this as a bug!", Toast.LENGTH_LONG)
.show();
state = State.INPUT;
updateView();
return;
}
}
} If you guys see anything fishy with that, let me know. I'll be looking at it. |
Also, it's going to take me a bit to get my test environment set back up here. If either of you have the app set up and can connect it to adb to pull the logs from the app during a send of one of these transactions, that would be helpful. I would expect to see the debug print above. |
Thanks Hank for starting to work on it! I will be setting up a test environment as well so I can contribute to the project for future releases/issues. That code snippet looks correct, so reviewing the logs should let us know what’s going on. From: hank [mailto:[email protected]] Also, it's going to take me a bit to get my test environment set back up here. If either of you have the app set up and can connect it to adb to pull the logs from the app during a send of one of these transactions, that would be helpful. I would expect to see the debug print above. — |
I took a look tonight, and tried to make a transaction that would cause the issue we see in the linked transaction. I have 10LTC in the app. If I try to send 9.999 it properly calculates the fee at 0.001 and asks me to confirm. If I do 9.9998, it disallows the transaction, claiming I need 0.0008 more to continue (which is true because we're not leaving enough for a tx fee there). I think the linked transaction was done with a previous version of the wallet or another client, because the current version doesn't seem to have this issue. There was a small bug when importing really old private keys (I have one from early 2013 that I was using for this) where the timestamp was set to the current time and the blockchain reset always depended on checkpoints if all of the keys had set timestamps. I went ahead and set all imported private keys from QR scans to a creation time of 0 so when you do a blockchain reset it goes back to genesis to capture all the transactions. It would be nice to be able to select the block height to go back to and find the first checkpoint before that height, but that isn't strictly necessary. I'll go ahead and test and commit this change because I tested it and it worked. Let me know what you guys think we should do regarding this ticket. |
I'm suffering this issue, but instead of delay I have a transaction totally freezed due to low fee. This is happening in a really old private key but with an updated client. Could we get an option to abandon these transactions as they are not going to be confirmed at any time? Thanks! |
In this case, you can try making a backup of your wallet and either importing it into another copy of the app (different phone or uninstall/reinstall the app AFTER MAKING THE BACKUP), or importing the backed up keys into a completely different client. The second option is what I recommend. There are instructions here on how to dump the keys from the backup: http://litecoinlearner.com/androidwallethelp.asp |
Thanks! |
Hey Hank,
As discussed via email, there appears to be an issue with the fee calculation when someone sends an amount to an address, and creates an output which is less then Litecoin Core's dust threshold value (0.001 LTC).
An example of a troublesome transaction can be seen below:
http://ltc.block-explorer.com/tx/6080e37e9eb509e4e732b590993ce91cd5a0fbbc5cb319f1da7ea63c3fa0d947
In this transaction, an output was created with a value of 0.00023934 LTC, thus being flagged as a dust transaction. Litecoin Core expects a 1000 byte penalty for every output below the dust threshold. After this, it also rounds up the transaction to the nearest 1000. Code for the penalty can be seen here:
https://github.com/litecoin-project/litecoin/blob/master-0.10/src/main.cpp#L917
And for the rounding up calculation:
https://github.com/litecoin-project/litecoin/blob/master-0.10/src/amount.cpp#L18
For this transaction, an error message was created when accepting it into the mempool on Litecoin Core:
ERROR: AcceptToMemoryPool : not enough fees 6080e37e9eb509e4e732b590993ce91cd5a0fbbc5cb319f1da7ea63c3fa0d947, 100000 < 200000
I'd imagine a solution to this problem would be:
a) If an output is made with an amount less then the dust threshold, add it to the transaction fee.
b) Add a fee penalty for every transaction output which is below the dust threshold.
The text was updated successfully, but these errors were encountered: