bitshares / bitshares/bitshares-core
fork_db.set_max_size called too early, may make switching forks impossible
- Dominant language
- C++
- Stars
- 1.2k
- Forks
- 660
- Avg merge
- 8h 17m
- Merged PRs (30d)
- 26
Description
**Bug Description**
This is an issue to track the scenario reported by steamit at https://github.com/steemit/steem/issues/2911 .
**Impacts**
- [ ] API (the application programming interface)
- [ ] Build (the build process or something prior to compiled code)
- [ ] CLI (the command line wallet)
- [ ] Deployment (the deployment process after building such as Docker, Travis, etc.)
- [ ] DEX (the Decentralized EXchange, market engine, etc.)
- [ ] P2P (the peer-to-peer network for transaction/block propagation)
- [ ] Performance (system or user efficiency, etc.)
- [x] Protocol (the blockchain logic, consensus, validation, etc.)
- [ ] Security (the security of system or user data, etc.)
- [ ] UX (the User Experience)
- [ ] Other (please add below)
**Steps To Reproduce**
As reported by Steamit (cut and pasted, as I tired of switching back and forth):
The incident occurred on 2018-09-17 when witnesses trying to switch back to 0.19.
- the node happily pushed 500 blocks (A1-A500) from the v19 chain to fork db, head is A500;
- since participation rate is low, LIB didn't advance, fork_db.max_size == 500, fork_db.head==A500 (assume A1 is next to LIB);
- Here comes a new array of blocks (B1 - B501) from the v20 chain, with highest block number greater than A500, all blocks can link back to the same LIB (assume B1 is next to LIB);
- The node happily pushed B1 - B500 into fork db, fork_db.head didn't change, _push_block() returns false right away;
- fork db is in a good state so far; fork_db.max_size == 500, fork_db.head==A500;
- the node pushes B501.
- Firstly it's pushed to fork_db, now fork_db.head==B501
- Since it links back to LIB, the node firstly popped A500-A1,
- fork_database::pop_block() is called 500 times,fork_db.head changed to B1;
- fork_db.max_size didn't change, is still 500;
- the node tries to push B1, triggered an assertion failure;
- B1 - B501 are removed from fork_db;
- `_fork_db.set_head( branches.second.front() );` is called, now fork_db.head is A500;
- the node pops block in the B chain until LIB (it's a no-op in the scenario described in this issue, but can occur in other scenarios, which changes fork_db.head)
- the node re-pushes A1-A500;
- when pushing A1, calling update_last_irreversible_block(), assume that db.head-db.lib+1=20, so fork_db.set_max_size(20) is called, so almost all blocks in the fork db got removed, but A481 to A500 are kept.
- the node pushes the other 499 blocks,
- fork_db.max_size == 500;
- fork_db.size() == 20, aka only 20 blocks (A481 to A500) inside;
- The node tries to generate a new block.
- LIB advances
- the node tries to get A1 as new LIB from fork_db and write it to block_log, but can't find it.
Possible solution:
- move fork_db.set_max_size() out of apply_block(), e.g. put it in push_block(), after resolved chain reorganizationsuccessfully pushed the block; and/or
- carefully maintain fork_db.head
**Expected Behavior**
A chain reorganization should be possible regardless of when an exception is thrown.
**Additional Context (optional)**
Suggested procedure: A careful walkthrough of the issue reported by steam should be done, preferably by a unit test. That should flush out the "steps to reproduce" and also validate the resolution once complete.
We have yet to verify the problem actually exists in the BitShares consensus logic.
Related: Issue #1333 is related, but not the same. It was determined that #1333 is not a problem in BitShares. Some of the code written to test #1333 may be reusable for this issue, and can be found in the branch jmj_1333.
## CORE TEAM TASK LIST
- [ ] Evaluate / Prioritize Bug Report
- [ ] Refine User Stories / Requirements
- [ ] Define Test Cases
- [ ] Design / Develop Solution
- [ ] Perform QA/Testing
- [ ] Update Documentation
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.