Bugfix: Initialize CWallet::nOrderPosNext on an empty wallet, and save it in db #1800

pull luke-jr wants to merge 1 commits into bitcoin:master from luke-jr:refactor_times changing 6 files +28 −4

luke-jr commented at 4:56 AM on September 8, 2012: member

Thanks @gmaxwell for finding these bugs!
Bugfix: Initialize CWallet::nOrderPosNext on an empty wallet, and save it in db da7b8c1260

in src/wallet.h:None in da7b8c1260

 145 | @@ -144,6 +146,11 @@ class CWallet : public CCryptoKeyStore
 146 |      bool ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase);
 147 |      bool EncryptWallet(const SecureString& strWalletPassphrase);
 148 |  
 149 | +    /** Increment the next transaction order id
 150 | +        @return next transaction order id

laanwj commented at 5:23 AM on September 8, 2012:

My head has difficulties parsing this English early in the morning. Maybe something like "Returns the next sequential transaction order id" would be clearer? I think the increment is implicit in "next".

BitcoinPullTester commented at 12:04 PM on September 8, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/da7b8c1260d91e3306eb18dd65633567cb31332f for binaries and test log.
gavinandresen commented at 6:17 PM on September 8, 2012: contributor

What is the severity of this bug? Does it prevent bitcoin from working at all when starting with an empty wallet?
gmaxwell commented at 6:48 PM on September 8, 2012: contributor

The position counter is uninitialized. So far I've never been able able to get it to a case where it happened to have any value other than zero, which is the correct and expected value for a new wallet. No matter what value it takes it should not be able to prevent bitcoin from working. The worst result would be some transactions appearing out of order.

I found the issues while making a second code review of all the later pulls, I don't believe any problems have been reported related to this.
luke-jr commented at 8:40 PM on September 8, 2012: member

In theory, a new wallet might put items out of order across restarts since they won't run the "process old data" function :/
luke-jr commented at 5:19 AM on September 11, 2012: member

bump, rc3?
gavinandresen merged this on Sep 18, 2012
gavinandresen closed this on Sep 18, 2012
luke-jr deleted the branch on Jan 1, 2015
sidhujag referenced this in commit e1172ca545 on Jan 2, 2018
DrahtBot locked this on Sep 8, 2021

Contributors