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

IBD: do not check PoW (Yespower) during Download headers #122

Merged
merged 3 commits into from May 9, 2020

Conversation

decryp2kanon
Copy link
Contributor

@decryp2kanon decryp2kanon commented May 5, 2020

Background

#119

TL;DR

  • CURRENT:
    • when downloading headers, check header (PoW: Yespower)
    • bottle neck at downloading headers
  • PR:
    • skip check header (PoW: Yespower), only in downloading headers
    • bottle neck gone! 🎉

Safe to skip?

CheckProofOfWork() is already multiple checking in several place. i believe that even if our node is attacked, it will defense well.


RESULT

  • ✔️ total IBD time ++23% faster
    image

  • ✔️ data traffic --60% reduced

    • before
      image

    • after
      image

  • ✔️ less CPU usage around --5% when IBD


What we are skipping?

skipped check CheckProofOfWork() at pow.cpp when Downloading headers

bool CheckProofOfWork(uint256 hash, unsigned int nBits, const Consensus::Params& params)
{
    bool fNegative;
    bool fOverflow;
    arith_uint256 bnTarget;

    bnTarget.SetCompact(nBits, &fNegative, &fOverflow);

    // Check range
    if (fNegative || bnTarget == 0 || fOverflow || bnTarget > UintToArith256(params.powLimit))
        return false;

    // Check proof of work matches claimed amount
    if (UintToArith256(hash) > bnTarget)
        return false;

    return true;
}

How to skip and when?

do not PoW check when Downloading headers of IBD (include fImporting and fReindex). if not, we do not skip.

if ( IsInitialBlockDownload() == false )

Safe enough?

i believe so. its already multiple checked in CheckBlock() and this can be found in 4 places:

CLICK ME

    1. AcceptBlock()
    if (!CheckBlock(block, state, chainparams.GetConsensus()) ||
        !ContextualCheckBlock(block, state, chainparams.GetConsensus(), pindex->pprev)) {
        if (state.IsInvalid() && !state.CorruptionPossible()) {
            pindex->nStatus |= BLOCK_FAILED_VALID;
            setDirtyBlockIndex.insert(pindex);
        }
        return error("%s: %s", __func__, FormatStateMessage(state));
    }
    1. TestBlockValidity()
    // NOTE: CheckBlockHeader is called by CheckBlock
    if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime()))
        return error("%s: Consensus::ContextualCheckBlockHeader: %s", __func__, FormatStateMessage(state));
    if (!CheckBlock(block, state, chainparams.GetConsensus(), fCheckPOW, fCheckMerkleRoot))
        return error("%s: Consensus::CheckBlock: %s", __func__, FormatStateMessage(state));
    1. VerifyDB()
        // check level 1: verify block validity
        if (nCheckLevel >= 1 && !CheckBlock(block, state, chainparams.GetConsensus()))
            return error("%s: *** found bad block at %d, hash=%s (%s)\n", __func__,
                         pindex->nHeight, pindex->GetBlockHash().ToString(), FormatStateMessage(state));
    1. ConnectBlock()
    // Check it again in case a previous version let a bad block in
    // NOTE: We don't currently (re-)invoke ContextualCheckBlock() or
    // ContextualCheckBlockHeader() here. This means that if we add a new
    // consensus rule that is enforced in one of those two functions, then we
    // may have let in a block that violates the rule prior to updating the
    // software, and we would NOT be enforcing the rule here. Fully solving
    // upgrade from one software version to the next after a consensus rule
    // change is potentially tricky and issue-specific (see RewindBlockIndex()
    // for one general approach that was used for BIP 141 deployment).
    // Also, currently the rule against blocks more than 2 hours in the future
    // is enforced in ContextualCheckBlockHeader(); we wouldn't want to
    // re-enforce that rule here (at least until we make it impossible for
    // GetAdjustedTime() to go backward).
    if (!CheckBlock(block, state, chainparams.GetConsensus(), !fJustCheck, !fJustCheck))
        return error("%s: Consensus::CheckBlock: %s", __func__, FormatStateMessage(state));

Screenshot during IBD

    1. This means PoW check during IBD is not actually skipped, but still its checking.
      image

@decryp2kanon decryp2kanon changed the title fix: do not PoW check when IBD for faster sync fix: do not PoW check during IBD May 5, 2020
@decryp2kanon decryp2kanon changed the title fix: do not PoW check during IBD fix: do not PoW check during Download headers of IBD May 5, 2020
@decryp2kanon decryp2kanon force-pushed the fix_IBD_header_2020-05-05 branch 2 times, most recently from 329c77a to 24313c5 Compare May 5, 2020 20:53
@decryp2kanon decryp2kanon requested a review from volbil May 5, 2020 23:13
@decryp2kanon decryp2kanon linked an issue May 5, 2020 that may be closed by this pull request
@decryp2kanon

This comment has been minimized.

@decryp2kanon
Copy link
Contributor Author

decryp2kanon commented May 6, 2020

when do PoW check?

  1. IBD to first user ✔️
  2. reindex-chainstate (fReindex) ✔️
  3. reindex (fReindex) ✔️
  4. fImporting ❓

image

@decryp2kanon decryp2kanon changed the title fix: do not PoW check during Download headers of IBD IBD: do not check PoW (Yespower) during Download headers May 6, 2020
This means PoW check during IBD is not actually skipped, but still its checking in another places. What we skipped is only when Downloading headers, but not else. This makes IBD much faster.
@decryp2kanon

This comment has been minimized.

@decryp2kanon

This comment has been minimized.

@akshaynexus
Copy link

Looks like a good change to me,i had alot of delay and cpu usage when syncing

@decryp2kanon

This comment has been minimized.

@decryp2kanon
Copy link
Contributor Author

Tested ACK 032184c

@decryp2kanon
Copy link
Contributor Author

decryp2kanon commented May 9, 2020

IBD around 10~30% faster, however it does not skip checking PoW, but only remove duplicated at downloading stage

@decryp2kanon
Copy link
Contributor Author

@decryp2kanon

This comment has been minimized.

Copy link
Member

@volbil volbil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good to me 👍

@decryp2kanon decryp2kanon merged commit 1d5a8c9 into master-v0.16.3 May 9, 2020
@decryp2kanon decryp2kanon deleted the fix_IBD_header_2020-05-05 branch May 9, 2020 14:42
decryp2kanon added a commit that referenced this pull request May 12, 2020
**Changes: v0.16.3.31rc1**

- Major
  * Fix: IBD `30%` faster, and `60%` reduced data traffic #122 
  * Add: new option `-prunedebuglogfile`: limit filesize of debug.log #110  
  * Add: when IBD, print blockheader count on debug.log #128 
  * Update: checkpoints (mainnet) #132 

- Minor
  * GUI: display size in MB (was GB) #125 
  * Revert: IBD settings back to BTC original #124 #126
decryp2kanon added a commit that referenced this pull request May 15, 2020
**Changes: v0.16.3.32rc2**

- Major
  * Fix: IBD `30%` faster, and `60%` reduced data traffic #122 
  * Add: new option `-prunedebuglogfile`: limit filesize of debug.log #110  
  * Add: when IBD, print blockheader count on debug.log #128 
  * Update: checkpoints (mainnet) #132 
  * Fix: IBD optimizing #135 

- Minor
  * GUI: display size in MB (was GB) #125 
  * Revert: IBD settings back to BTC original #124 #126
decryp2kanon added a commit that referenced this pull request May 23, 2020
**Changes: v33rc3**

- Major
  * Fix: IBD `30%` faster, and `60%` reduced data traffic #122 
  * Add: new option `-prunedebuglogfile`: limit filesize of debug.log #110  
  * Add: when IBD, print blockheader count on debug.log #128 
  * Update: checkpoints (mainnet) #132 
  * Fix: IBD optimizing #135 
  * Update: seed list #144 
  * Remove: BCLog::POW (-debug=pow) #142 

- Minor
  * GUI: display size in MB (was GB) #125 
  * Revert: IBD settings back to BTC original #124 
  * Add: bootstrap height at 4421701 #143 
  * Fix: travis pathlib2 #148
decryp2kanon added a commit that referenced this pull request Jun 6, 2020
**Changes: v34-starboy (same as v33)**

- Major
  * Fix: IBD `30%` faster, and `60%` reduced data traffic #122 
  * Add: new option `-prunedebuglogfile`: limit filesize of debug.log #110  
  * Add: when IBD, print blockheader count on debug.log #128 
  * Update: checkpoints (mainnet) #132 
  * Fix: IBD optimizing #135 
  * Update: seed list #144 
  * Remove: BCLog::POW (-debug=pow) #142 

- Minor
  * GUI: display size in MB (was GB) #125 
  * Revert: IBD settings back to BTC original #124 
  * Add: bootstrap height at 4421701 #143 
  * Fix: travis pathlib2 #148
decryp2kanon added a commit that referenced this pull request Jun 14, 2020
* IBD: do not check PoW (Yespower) during downloading headers
However this means checking PoW during IBD is, not actually skipped, but still checking in another places. This makes IBD much faster.
* remove: debug printf
* adding comment by volbil
decryp2kanon added a commit that referenced this pull request Jun 14, 2020
**Changes: v0.16.3.31rc1**

- Major
  * Fix: IBD `30%` faster, and `60%` reduced data traffic #122 
  * Add: new option `-prunedebuglogfile`: limit filesize of debug.log #110  
  * Add: when IBD, print blockheader count on debug.log #128 
  * Update: checkpoints (mainnet) #132 

- Minor
  * GUI: display size in MB (was GB) #125 
  * Revert: IBD settings back to BTC original #124 #126
decryp2kanon added a commit that referenced this pull request Jun 14, 2020
**Changes: v0.16.3.32rc2**

- Major
  * Fix: IBD `30%` faster, and `60%` reduced data traffic #122 
  * Add: new option `-prunedebuglogfile`: limit filesize of debug.log #110  
  * Add: when IBD, print blockheader count on debug.log #128 
  * Update: checkpoints (mainnet) #132 
  * Fix: IBD optimizing #135 

- Minor
  * GUI: display size in MB (was GB) #125 
  * Revert: IBD settings back to BTC original #124 #126
decryp2kanon added a commit that referenced this pull request Jun 14, 2020
**Changes: v33rc3**

- Major
  * Fix: IBD `30%` faster, and `60%` reduced data traffic #122 
  * Add: new option `-prunedebuglogfile`: limit filesize of debug.log #110  
  * Add: when IBD, print blockheader count on debug.log #128 
  * Update: checkpoints (mainnet) #132 
  * Fix: IBD optimizing #135 
  * Update: seed list #144 
  * Remove: BCLog::POW (-debug=pow) #142 

- Minor
  * GUI: display size in MB (was GB) #125 
  * Revert: IBD settings back to BTC original #124 
  * Add: bootstrap height at 4421701 #143 
  * Fix: travis pathlib2 #148
decryp2kanon added a commit that referenced this pull request Jun 14, 2020
**Changes: v34-starboy (same as v33)**

- Major
  * Fix: IBD `30%` faster, and `60%` reduced data traffic #122 
  * Add: new option `-prunedebuglogfile`: limit filesize of debug.log #110  
  * Add: when IBD, print blockheader count on debug.log #128 
  * Update: checkpoints (mainnet) #132 
  * Fix: IBD optimizing #135 
  * Update: seed list #144 
  * Remove: BCLog::POW (-debug=pow) #142 

- Minor
  * GUI: display size in MB (was GB) #125 
  * Revert: IBD settings back to BTC original #124 
  * Add: bootstrap height at 4421701 #143 
  * Fix: travis pathlib2 #148
decryp2kanon added a commit that referenced this pull request Jun 14, 2020
* add: option "-prunedebuglogfile": limit filesize of debug.log (#110)

The idea is `ShrinkDebugFile` in realtime.

- AIM:
to prevent disk is filling full up with log file.

- DEBUG:
If `debug.log` is over 10 MB (`10*1000*1000`), shrink to 1 MB (`1*1000*1000`). 10x smaller
  * `watch -n1 ls -lh debug.log`
  * `watch -n5 ps -p "$(cat sugarchaind.pid)" -o %cpu,%mem,cmd`

- RUN: 
check logging speed and filesize
  * `sugarchaind -prunedebuglogfile -reindex-chainstate`

- PERIOD:
a cycle took around `4:30` when `-reindex-chainstate`
```
2020-04-20 23:18:55 DEBUG.LOG PRUNED at 10000071
2020-04-20 23:24:33 DEBUG.LOG PRUNED at 10000014
2020-04-20 23:30:11 DEBUG.LOG PRUNED at 10000018
2020-04-20 23:35:45 DEBUG.LOG PRUNED at 10000186
```

* IBD: do not check PoW (Yespower) during downloading headers (#122)

* IBD: do not check PoW (Yespower) during downloading headers
However this means checking PoW during IBD is, not actually skipped, but still checking in another places. This makes IBD much faster.
* remove: debug printf
* adding comment by volbil

* revert: (#80) do not disconnect whitelisted peers during IBD (#124)

* GUI: do not display in GB, but in MB (#125)

* revert (#78) & fix: disabled more getheaders (#126)

* revert&fix: disabled more getheaders
* remove: printf

* IBD: Print blockheader count on debug.log (#128)

* revert: MINIMUM_CONNECT_TIME (#129)

https://github.com/bitcoin/bitcoin/blob/f56c00b2345cd2e392ade4733e2ca9cb9b0af623/src/net_processing.h#L36

* Revert "revert: MINIMUM_CONNECT_TIME (#129)" (#130)

This reverts commit e37dfec.

* update: checkpoint (#132)

* bump 0.16.3.31rc1 + manpage (#133)

**Changes: v0.16.3.31rc1**

- Major
  * Fix: IBD `30%` faster, and `60%` reduced data traffic #122 
  * Add: new option `-prunedebuglogfile`: limit filesize of debug.log #110  
  * Add: when IBD, print blockheader count on debug.log #128 
  * Update: checkpoints (mainnet) #132 

- Minor
  * GUI: display size in MB (was GB) #125 
  * Revert: IBD settings back to BTC original #124 #126

* IBD: max blocks in transit per peer (cached PoW) (#135)

* bump v0.16.3.32rc2 + manpage (#137)

**Changes: v0.16.3.32rc2**

- Major
  * Fix: IBD `30%` faster, and `60%` reduced data traffic #122 
  * Add: new option `-prunedebuglogfile`: limit filesize of debug.log #110  
  * Add: when IBD, print blockheader count on debug.log #128 
  * Update: checkpoints (mainnet) #132 
  * Fix: IBD optimizing #135 

- Minor
  * GUI: display size in MB (was GB) #125 
  * Revert: IBD settings back to BTC original #124 #126

* fix: daemon Killed on ARM during IBD (out-of-memory) (#140)

* Revert "revert (#78) & fix: disabled more getheaders (#126)"

This reverts commit 7c45e62.

* comment

* remove: BCLog::POW (-debug=pow) (#142)

* add: bootstrap height at 4421701 (#143)

* scripts: In linearize, search for next position of magic bytes rather than fail
bitcoin/bitcoin#16802

* add: bootstrap height at 4421701

* update: seeds 2020-05-19 KST (#144)

* fix: travis: pathlib2 (#148)

ImportError: No module named 'pathlib2'

* doc (#151)

* bump v0.16.3.33rc3 + manpage (#152)

**Changes: v33rc3**

- Major
  * Fix: IBD `30%` faster, and `60%` reduced data traffic #122 
  * Add: new option `-prunedebuglogfile`: limit filesize of debug.log #110  
  * Add: when IBD, print blockheader count on debug.log #128 
  * Update: checkpoints (mainnet) #132 
  * Fix: IBD optimizing #135 
  * Update: seed list #144 
  * Remove: BCLog::POW (-debug=pow) #142 

- Minor
  * GUI: display size in MB (was GB) #125 
  * Revert: IBD settings back to BTC original #124 
  * Add: bootstrap height at 4421701 #143 
  * Fix: travis pathlib2 #148

* update: blockchain size as 3GB (#153)

* cleanup (#154)

* comment
* seeds version checker

* doc: release note v34 starboy (#160)

* doc: release note: starboy
* fix: known issue

* bump: v0.16.3.34-starboy + manpage (#161)

**Changes: v34-starboy (same as v33)**

- Major
  * Fix: IBD `30%` faster, and `60%` reduced data traffic #122 
  * Add: new option `-prunedebuglogfile`: limit filesize of debug.log #110  
  * Add: when IBD, print blockheader count on debug.log #128 
  * Update: checkpoints (mainnet) #132 
  * Fix: IBD optimizing #135 
  * Update: seed list #144 
  * Remove: BCLog::POW (-debug=pow) #142 

- Minor
  * GUI: display size in MB (was GB) #125 
  * Revert: IBD settings back to BTC original #124 
  * Add: bootstrap height at 4421701 #143 
  * Fix: travis pathlib2 #148
decryp2kanon added a commit to sugarchain-project/yumekawa that referenced this pull request Nov 2, 2020
- Changes:
  * During IBD, do not download additional block headers, against too much traffic
  * Skip BIP30 // See: https://github.com/litecoin-project/litecoin/blob/81c4f2d80fbd33d127ff9b31bf588e4925599d79/src/validation.cpp#L1866
  * During download headers, do not CheckBlockHeader for performance reason // See sugarchain-project/sugarchain#122
  * Rename: Testnet v3 -> v5 // See sugarchain-project/sugarchain@89cf9b5
  * During IBD, do not print "Potential stale tip detected..."
    - Disabled Tests:
      * feature_uacomment.py
      * feature_asmap.py

- Fixed Tests:
  * MAX_MONEY
decryp2kanon added a commit to sugarchain-project/yumekawa that referenced this pull request Nov 3, 2020
- Changes:
  * During IBD, do not download additional block headers, against too much traffic
  * Skip BIP30 // See: https://github.com/litecoin-project/litecoin/blob/81c4f2d80fbd33d127ff9b31bf588e4925599d79/src/validation.cpp#L1866
  * During download headers, do not CheckBlockHeader for performance reason // See sugarchain-project/sugarchain#122
  * Rename: Testnet v3 -> v5 // See sugarchain-project/sugarchain@89cf9b5
  * During IBD, do not print "Potential stale tip detected..."
    - Disabled Tests:
      * feature_uacomment.py
      * feature_asmap.py

- Fixed Tests:
  * MAX_MONEY
decryp2kanon added a commit to sugarchain-project/yumekawa that referenced this pull request Nov 6, 2020
- Changes:
  * During IBD, do not download additional block headers, against too much traffic
  * Skip BIP30 // See: https://github.com/litecoin-project/litecoin/blob/81c4f2d80fbd33d127ff9b31bf588e4925599d79/src/validation.cpp#L1866
  * During download headers, do not CheckBlockHeader for performance reason // See sugarchain-project/sugarchain#122
  * Rename: Testnet v3 -> v5 // See sugarchain-project/sugarchain@89cf9b5
  * During IBD, do not print "Potential stale tip detected..."
    - Disabled Tests:
      * feature_uacomment.py
      * feature_asmap.py

- Fixed Tests:
  * MAX_MONEY
decryp2kanon added a commit to sugarchain-project/yumekawa that referenced this pull request Nov 6, 2020
- Changes:
  * During IBD, do not download additional block headers, against too much traffic
  * Skip BIP30 // See: https://github.com/litecoin-project/litecoin/blob/81c4f2d80fbd33d127ff9b31bf588e4925599d79/src/validation.cpp#L1866
  * During download headers, do not CheckBlockHeader for performance reason // See sugarchain-project/sugarchain#122
  * Rename: Testnet v3 -> v5 // See sugarchain-project/sugarchain@89cf9b5
  * During IBD, do not print "Potential stale tip detected..."
    - Disabled Tests:
      * feature_uacomment.py
      * feature_asmap.py

- Fixed Tests:
  * MAX_MONEY
decryp2kanon added a commit to sugarchain-project/yumekawa that referenced this pull request Nov 16, 2020
- Changes:
  * During IBD, do not download additional block headers, against too much traffic
  * Skip BIP30 // See: https://github.com/litecoin-project/litecoin/blob/81c4f2d80fbd33d127ff9b31bf588e4925599d79/src/validation.cpp#L1866
  * During download headers, do not CheckBlockHeader for performance reason // See sugarchain-project/sugarchain#122
  * Rename: Testnet v3 -> v5 // See sugarchain-project/sugarchain@89cf9b5
  * During IBD, do not print "Potential stale tip detected..."
    - Disabled Tests:
      * feature_uacomment.py
      * feature_asmap.py

- Fixed Tests:
  * MAX_MONEY
decryp2kanon added a commit to sugarchain-project/yumekawa that referenced this pull request Nov 20, 2020
- Changes:
  * During IBD, do not download additional block headers, against too much traffic
  * Skip BIP30 // See: https://github.com/litecoin-project/litecoin/blob/81c4f2d80fbd33d127ff9b31bf588e4925599d79/src/validation.cpp#L1866
  * During download headers, do not CheckBlockHeader for performance reason // See sugarchain-project/sugarchain#122
  * Rename: Testnet v3 -> v5 // See sugarchain-project/sugarchain@89cf9b5
  * During IBD, do not print "Potential stale tip detected..."
    - Disabled Tests:
      * feature_uacomment.py
      * feature_asmap.py

- Fixed Tests:
  * MAX_MONEY
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

too much traffic during IBD
5 participants