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

Fixing transaction sync when recovering #807

Merged
merged 5 commits into from Dec 30, 2019
Merged

Fixing transaction sync when recovering #807

merged 5 commits into from Dec 30, 2019

Conversation

@levonpetrosyan93
Copy link
Contributor

levonpetrosyan93 commented Dec 24, 2019

No description provided.

@levonpetrosyan93 levonpetrosyan93 requested review from a-bezrukov and riordant Dec 24, 2019
pindex = chainActive.Next(pindex);
// if you are recovering wallet with mnemonics start rescan from block when mnemonics implemented in Zcoin
if (fRecoverMnemonic)
pindex = chainActive[222400];

This comment has been minimized.

Copy link
@a-bezrukov

a-bezrukov Dec 25, 2019

Contributor

It may be the testnet and it may be syncing from scratch (block < 222400)

This comment has been minimized.

Copy link
@levonpetrosyan93

levonpetrosyan93 Dec 25, 2019

Author Contributor

yes, correct, it was an quick push, to make it reviewable, will fix it if no other comment

This comment has been minimized.

Copy link
@psolstice

psolstice Dec 25, 2019

Contributor

also what if there are fewer blocks synced in the chain? and why previous section is deleted?

This comment has been minimized.

Copy link
@levonpetrosyan93

levonpetrosyan93 Dec 26, 2019

Author Contributor

in that case it like ignores -rescan and continues syncing, about second question no need to scan from genesis block as if you are recovering with mnemonic than your wallet is created after specified block,

pindex = chainActive.Next(pindex);
// if you are recovering wallet with mnemonics start rescan from block when mnemonics implemented in Zcoin
if (fRecoverMnemonic)
pindex = chainActive[222400];

This comment has been minimized.

Copy link
@psolstice

psolstice Dec 25, 2019

Contributor

also what if there are fewer blocks synced in the chain? and why previous section is deleted?

@@ -8152,7 +8156,10 @@ bool CWallet::InitLoadWallet() {
LogPrintf("Rescanning last %i blocks (from block %i)...\n", chainActive.Height() - pindexRescan->nHeight,
pindexRescan->nHeight);
nStart = GetTimeMillis();
walletInstance->ScanForWalletTransactions(pindexRescan, true);
bool fRecoverMnemonic = false;
if (GetBoolArg("-fRecoverMnemonic", false))

This comment has been minimized.

Copy link
@psolstice

psolstice Dec 25, 2019

Contributor

all the other possible command line arguments look in specific way, this has to be renamed

@reubenyap reubenyap added this to the 13.8.10 milestone Dec 25, 2019
@reubenyap reubenyap added this to Needs review in Zcoin Core via automation Dec 25, 2019
@reubenyap reubenyap added the bug label Dec 25, 2019
pindex = chainActive.Next(pindex);
// if you are recovering wallet with mnemonics start rescan from block when mnemonics implemented in Zcoin
if (fRecoverMnemonic)
pindex = chainActive[chainParams.GetConsensus().nMnemonicBlock];

This comment has been minimized.

Copy link
@psolstice

psolstice Dec 27, 2019

Contributor

I don't like that here there is no check if nMnemonicBlock is less than current height. Looking at the code it seems like it will work anyway (operator [] will return null and then everything is ok if pindex is null). But in case of subsequent changes to the code it might lead to unpleasant bug. Can we set pindex to chain tip if that's the case?

This comment has been minimized.

Copy link
@levonpetrosyan93

levonpetrosyan93 Dec 29, 2019

Author Contributor

it will be ok if we set chain tip in case of null.

Copy link
Contributor

riordant left a comment

See #808

Remove unnecessary bool arg (Fix for #807)
@levonpetrosyan93 levonpetrosyan93 requested review from riordant and psolstice Dec 29, 2019
Copy link
Contributor

riordant left a comment

LGTM

Zcoin Core automation moved this from Needs review to Reviewer approved Dec 30, 2019
@levonpetrosyan93 levonpetrosyan93 merged commit 1d669fe into master Dec 30, 2019
4 checks passed
4 checks passed
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No code changes detected
Details
LGTM analysis: C/C++ No new or fixed alerts
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
Zcoin Core automation moved this from Reviewer approved to Done Dec 30, 2019
@a-bezrukov a-bezrukov deleted the sync_fix branch Dec 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Zcoin Core
  
Done
5 participants
You can’t perform that action at this time.