Skip to content
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

Fund calculation inconsistent #582

Closed
harikattar opened this issue Jan 6, 2018 · 8 comments
Closed

Fund calculation inconsistent #582

harikattar opened this issue Jan 6, 2018 · 8 comments

Comments

@harikattar
Copy link

$ grin wallet -p password info

Wallet Summary Info - Block Height: 16539
  --------------------------
  Total                            | 75900.039000000 
  Awaiting Confirmation            | 0.000000000 
  Confirmed but Still Locked       | 10450.036000000 
  Currently Spendable              | 65450.003000000 
  ---------                        | --------- 
  (Locked by previous transaction) | 0.000000000 

$ grin wallet -p password send 60000 > send.tx
Jan 06 03:46:23.695 ERRO Tx not sent: insufficient funds (max: 26950.003000000)

That seems pretty wildly divergent.

That's also like 1/10th of all available grin if I'm calculating right - so I'm guessing it's not properly culling rewards that got orphaned by the main chain.

@harikattar
Copy link
Author

It's actually a misleading error message. Would be better phrased as "Too many inputs" or
something.

That wallet is almost entirely composed of 50-grin rewards, so that's 500+ inputs in a single TX. I was able to send it in three transactions, then those three into one for the 60000 I was trying to move in the first place.

@heunglee
Copy link
Contributor

heunglee commented Jan 6, 2018

I will see what I do for this issue.

@heunglee
Copy link
Contributor

heunglee commented Jan 6, 2018

Currently max outputs for each transaction of sending is 500 and hardcoded in the source. So if the amount is more than total amount of 500 UTXO's queried from wallet data, the wallet issues the error of insufficient funds.

Options of solution:

  1. Send total coins for the input amount in a transaction.
  2. if max outputs should not be changed, then query all eligible coins and send the input amount as a sequence of transactions.

Impacts:

  • Changes in querying wallet data. currently set to 500 result sets.
  • For option 2, create multiple transactions to send amount more than 500 eligible coins.
  • more memory consumption for larger amounts which may cause heap exhaustion?

@ignopeverell What is your suggestion?

@ignopeverell
Copy link
Contributor

We can't do 2, that would be too unexpected for the user. The limit exists because by default, we always select as many inputs as possible in a transaction, to reduce both the UTXO set and the fees. But that only makes sense up to a point, hence the limit to avoid being too greedy.

But if 500 is actually not enought to cover the whole amount, I think we should allow going over it to satisfy what the user wants to send. So I'd consider it more of a soft limit.

@heunglee
Copy link
Contributor

heunglee commented Jan 7, 2018

Then the fix will be to change the query conditions of eligible outputs. The condition will be: if 500 inputs are not enough to cover the whole amount, the wallet increases the number of inputs to cover the whole in a transaction.

sesam added a commit to sesam/grin that referenced this issue Jan 10, 2018
@heunglee
Copy link
Contributor

I made a fix for it, but have not completed thorough tests before commit. I was going to complete it tonight. If your fix is ready to commit, you can go ahead.

@heunglee
Copy link
Contributor

It seems to me @sesam waits for me to commit the fix because of only reason I started the fix first. So I took his solution with minor modification. @sesam's help is much appreciated.

@sesam
Copy link
Contributor

sesam commented Jan 10, 2018

Good! You found the bug in mine :) I got sidetracked looking at https://doc.rust-lang.org/std/primitive.slice.html#method.sort_unstable_by_key and rust-lang/rust#34447 -- but better look at that long after Testnet2 is done.

ignopeverell pushed a commit that referenced this issue Jan 11, 2018
* Add 'sent' to wallet info output (#288)
* Fund calculation inconsistent (#582)
yeastplume pushed a commit that referenced this issue Jan 12, 2018
…l quantities #396 (#603)

* Fund calculation inconsistent (#582)

* [wallet] panic: 'attempt to subtract with overflow' when sending small quantities (#396)
yeastplume pushed a commit that referenced this issue Jan 12, 2018
…ow' when sending small quantities #396 (#604)

* Add 'sent' to wallet info output (#288)

* Fund calculation inconsistent (#582)

* [wallet] panic: 'attempt to subtract with overflow' when sending small quantities (#396)

* [wallet] panic: 'attempt to subtract with overflow' when sending small quantities (#396)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants