Skip to content

Conversation

@laanwj
Copy link
Member

@laanwj laanwj commented Jul 10, 2012

Fixes #1452.

Until we can make the logic water-tight and are notified in every case the balance might have changed (either due to blocks maturing, or outputs spent), remove the premature optimization and simply recompute the balance every half a second when the number of blocks changed.

Shouldn't make any impact on performance in practice.

@Diapolo
Copy link

Diapolo commented Jul 10, 2012

If this works very cool, currently preparing a test!

One thing (unrelated to the problem), could you harmonize the order in checkBalanceChanged() and updateTransaction() when the emit is called. Either first update the vars and then emit or first emit the signal and then update the vars. I dislike such small coding discrepances, can't help.

Fixes bitcoin#1452. Until we can make the logic water-tight *and* are notified in every
case the balance might have changed, remove the premature optimization and
simply recompute the balance every half a second when the number of blocks changed.
@laanwj
Copy link
Member Author

laanwj commented Jul 11, 2012

Changed the order. It indeed makes sense to first update the variables and then emit the signal, so that calls to getXXX in the signal handler return the new value.

@Diapolo
Copy link

Diapolo commented Jul 11, 2012

I currently can't verify as my testnet coins are immature, so I can't empty my wallet -_-.

@Diapolo
Copy link

Diapolo commented Jul 13, 2012

I made a test on testnet, which was succesfull a day ago and a few seconds ago on a mainnet wallet, which also works like expected. Amounts get cleanly updated.

ACK

laanwj added a commit that referenced this pull request Jul 13, 2012
(UI) Persistently poll for balance change when number of blocks changed
@laanwj laanwj merged commit 916b11f into bitcoin:master Jul 13, 2012
nifgraup pushed a commit to nifgraup/auroracoin-project that referenced this pull request Mar 30, 2014
…check

(UI) Persistently poll for balance change when number of blocks changed
@laanwj laanwj deleted the 2012_07_persistentbalancecheck branch April 9, 2014 14:18
suprnurd pushed a commit to chaincoin-legacy/chaincoin that referenced this pull request Dec 5, 2017
…1575) + Dashify

* Dont deserialize nVersion into CNode, should fix bitcoin#9212

* net: deserialize the entire version message locally

This avoids having some vars set if the version negotiation fails.

Also copy it all into CNode at the same site. nVersion and
fSuccessfullyConnected are set last, as they are the gates for the other vars.
Make them atomic for that reason.

* net: don't run callbacks on nodes that haven't completed the version handshake

Since ForEach* are can be used to send messages to  all nodes, the caller may
end up sending a message before the version handshake is complete. To limit
this, filter out these nodes. While we're at it, may as well filter out
disconnected nodes as well.

Delete unused methods rather than updating them.

* net: Disallow sending messages until the version handshake is complete

This is a change in behavior, though it's much more sane now than before.

* net: log an error rather than asserting if send version is misused

Also cleaned up the comments and moved from the header to the .cpp so that
logging headers aren't needed from net.h

* Implement conditions for ForEachNode() and ForNode() methods of CConnman.

A change making ForEachNode() and ForNode() methods ignore nodes that
have not completed initial handshake have been backported from Bitcoin.
Unfortunately, some Dash-specific code needs to iterate over all nodes.

This change introduces additional condition argument to these methods.
This argument is a functional object that should return true for nodes
that should be taken into account, not ignored.

Two functional objects are provided in CConnman namespace:
* FullyConnectedOnly returns true for nodes that have handshake completed,
* AllNodes returns true for all nodes.

Overloads for ForEachNode() and ForNode() methods without condition argument
are left for compatibility with non-Dash-specific code.
They use FullyConnectedOnly functional object for condition.

Signed-off-by: Oleg Girko <ol@infoserver.lv>

* Iterate over all nodes in Dash-specific code using AllNodes condition.

Use AllNodes functional object as newly introduced condition argument for
ForEachNode() and ForNode() methods of CConnman to iterate over all nodes
where needed in Dash-specific code.

Signed-off-by: Oleg Girko <ol@infoserver.lv>
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request May 6, 2020
08092dd [Bug][Wallet] Fix CWallet::GetMinimumFee improperly set to feePerKb (random-zebra)

Pull request description:

  nFeeNeeded is an absolute value (`feePerKb * txKb`), and, as explained both in GUI labels, and in the RPC help, `-payTxFee` is a value per Kb.
  `CWallet::GetMinimumFee` , instead, was improperly setting `nFeeNeeded` to just `feePerKb` when a user had a custom fee set, resulting in the creation of transactions with fee much higher than expected (most transactions are well under 1Kb in size).

  Closes bitcoin#881

ACKs for top commit:
  furszy:
    utACK 08092dd
  Fuzzbawls:
    ACK 08092dd

Tree-SHA512: bee4b0782879596e68fbc40c36fa4bda1fee7a04117e9487ed7f7bf580feb9785c518857e694702555315f5560fff92556ba39f7f6e4c0be46ad48309e5d6e29
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request May 6, 2020
df9fc17 [QA][Bug] Shorter wallet_basic.py functional test (random-zebra)

Pull request description:

  Our current `wallet_basic.py` test takes quite some time.
  Most of the preparations, though, are unnecessary, as we don't have the feature supposed to be checked (e.g. "subtract fee from amount", or fee collected by the miner or list zero value tx as available coins).
  Also restore the check for the correct fee amount, previously commented, with `assert_fee_amount`.
  Finally moves the `--zapwallettxes` checks to its test. This makes wallet_basic much faster (498 seconds vs 833) and wallet_zapwallettxes slower (but, at least, it can be run in parallel with the other, making the overall test_runner runtime shorter).

  Note: This also fixes a bug in the previous version of the test, highlighted by bitcoin#1575 (the fee was fixed at the value taken from the first tx sent).

ACKs for top commit:
  Fuzzbawls:
    ACK df9fc17
  furszy:
    ACK df9fc17

Tree-SHA512: a8775943aca1b85cb593e017dbd763f25617c8c9450688c615f6ba2867546d4a7ce82f23273336a91f3385dc1ffae18f679d0b0eca863fde84ce9e432c4d2881
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request May 6, 2020
…ng insane fees

b8fad43 [Trivial] Fix wording: "must be above" to "must be at least" (random-zebra)
03be61e [GUI] Custom fee, min amount validation. (furszy)
d063a7f [GUI] SendCustomFeeDialog: prevent user from saving insane fees (random-zebra)

Pull request description:

  Based on top of:
  - [x] bitcoin#1574
  - [x] bitcoin#1575
  - [x] bitcoin#1580

  Straightforward update, as per title.
  Closes bitcoin#1570

  EDIT: cherrypicked a commit from @furszy , for min fee validation too.

ACKs for top commit:
  furszy:
    ACK b8fad43
  Fuzzbawls:
    ACK b8fad43

Tree-SHA512: 87be1f59e82f144b8330b18aafedcdd46b8b2c340c29d2a8f21efe9293899643d4b564666e673d54cade8d0e4a6be0fd17b1f6e1af4eeb9a767e363a509e9612
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GUI: display error when sending all coins

2 participants