Repository navigation
Add method to check the end of a term - #1544
foriequal0 wants to merge 1 commit into
Conversation
135d20c to
a4cf28e
Compare
| client.db().write_buffered(batch); | ||
| chain.commit(); | ||
|
|
||
| self.check_term_end(block.header(), &chain, client); |
There was a problem hiding this comment.
This could make a problem.
In Tendermint Consensus, there will be proposal blocks which are executed but not finalized. Since the proposal blocks' timestamp will be different, some of it cause term end, and others do not cause term end. So the code, saving the "last term" to DB should not be called here. The code should be called when a block is finalized. And the finalization time would be "chain_notify".
@sgkim126 is handling a similar problem while implementing changing common params. Please check his work.
There was a problem hiding this comment.
Yes, it can be a problem when the block is a proposal block or there is a reorganization.
https://github.com/CodeChain-io/codechain/blob/master/core/src/client/client.rs#L150
To handle it, I extracted new_blocks method on the client.
I'm considering this method to check the params.
There was a problem hiding this comment.
However, you need to change Importer::import_verified_blocks to stop when the current block is the end of a term.
There was a problem hiding this comment.
I think I should change how the solo closes term. I think the solo should closes term still controllable manner (not non-deterministic time-based manner) but it is easily determined by just looking the header. Solo should closes term by block number base. The current manual trigger causes some problems. When the solo is configured with multiple nodes, they cannot agree when the term ends. and the differences between solo and the corgi make the problem harder such as how we deal with reorganization, when do we stop at on import_verified_blocks.
a4cf28e to
95ad0ef
Compare
sgkim126
changed the title
|
Let me close it. It's implemented in the other way. |
Depends on: #1537