bitshares / bitshares/bitshares-core
Now is not always now
- Dominant language
- C++
- Stars
- 1.2k
- Forks
- 660
- Avg merge
- 8h 17m
- Merged PRs (30d)
- 26
Description
**Bug Description**
When processing a block, we usually use `database::head_block_time()` aka `dynamic_global_properties.time` to indicate "now". We also use `database::head_block_num()`. However, these variables are updated in the middle of the process, specifically, in `database::update_global_dynamic_data()`, after processed transactions: https://github.com/bitshares/bitshares-core/blob/44a7c47a3c063033f535ee039cfd6b55088dce2a/libraries/chain/db_block.cpp#L520-L524
It means when processing transactions, we're using the time and block number of last block. Technically it is not a big deal, but it has brought some confusions, for example:
* if an object (e.g. account or limit order) is created in a block by a transaction, its creation time and block number stored in object database are same as the values of previous block, but not current block;
* a transaction would have expired, or would be invalid due to other time-related reasons (e.g. hard fork time check), but still can be included in a block (#356);
* a force-settlement will execute one block earlier;
* (perhaps) an order / proposal / withdraw_permission can be created in a block and then expire in the same block.
**Impacts**
Describe which portion(s) of BitShares Core may be impacted by this bug. Please tick at least one box.
- [ ] 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.)
- [x] UX (the User Experience)
- [ ] Other (please add below)
**Expected Behavior**
Block time and block number should be consistent when processing a block.
**Additional Context (optional)**
This change requires a consensus upgrade. Do we need to correct/update old data after the upgrade?
## 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
Research direction
Start in libraries/chain/db_block.cpp, especially database::update_global_dynamic_data(), and trace database::head_block_time() and database::head_block_num() while transactions are processed. Determine the consensus-upgrade design, compatibility with old data, and tests needed to verify current-block time and number behavior; done means both values are consistent without invalidating consensus.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- blockchain
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100