bitshares / bitshares/bitshares-core

Now is not always now

Open
#1,198 5 comments 0 reactions 0 assignees View on GitHub
bug hardfork informative
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.