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

add create empty blocks config for single consensus #432

Closed

Conversation

godeamon
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@zhugelianglongming zhugelianglongming left a comment

Choose a reason for hiding this comment

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

建议更正字段名,优化实现位置

example/xchain/conf/engine.yaml Outdated Show resolved Hide resolved
kernel/engines/xuperos/config/config.go Outdated Show resolved Hide resolved
if m.ctx.EngCtx.EngCfg.DisableEmyptBlocks && !m.ctx.State.HasUnconfirmTx() {
consensusStatus, err := m.ctx.Consensus.GetConsensusStatus()
if err != nil {
return err
Copy link
Collaborator

Choose a reason for hiding this comment

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

对于挖矿条件的判断,实现在m.mining(ctx)中可读性会不会更好一些?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

m.mining(ctx) 已经开始挖矿了,所以我放到外面了。

Copy link
Collaborator

@zhugelianglongming zhugelianglongming Feb 17, 2023

Choose a reason for hiding this comment

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

主要是有个疑问:

  • mining() 实现的第一部分为"共识挖矿前处理"的定义是什么?
  • 打包区块前的判断逻辑,根据什么判断要放在mining()的函数头部 还是 函数调用前?

Copy link
Collaborator

@zhugelianglongming zhugelianglongming Feb 17, 2023

Choose a reason for hiding this comment

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

空块判断不太算 ProcessBeforeMiner 的内容

节点若为矿工,首先通过Consensus模块ProcessBeforeMiner接口更新自己共识内存状态,准备开始生产区块。
5. 共识算法 — XuperChain 官方文档 文档

Copy link
Collaborator

@zhugelianglongming zhugelianglongming Feb 17, 2023

Choose a reason for hiding this comment

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

我有一点建议是:

  • 上层调用的时候,在确认是挖矿状态后,就只负责调用挖矿函数。
  • 至于挖矿的前置处理、特殊判断(比如空块判断)等,可以在挖矿过程中,分阶段处理。

更具体的,在这个 case,我可能会建议实现在m.packBlock
相当于打包过程中,对区块大小的上限和下限分别进行判断处理。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

共识这些接口开源文档里都有

Copy link
Collaborator

@zhugelianglongming zhugelianglongming left a comment

Choose a reason for hiding this comment

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

Functionally acceptable

@codecov-commenter
Copy link

Codecov Report

Merging #432 (7f55c97) into master (fa11371) will decrease coverage by 0.07%.
The diff coverage is 0.00%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master     #432      +/-   ##
==========================================
- Coverage   43.86%   43.80%   -0.07%     
==========================================
  Files         149      149              
  Lines       13358    13359       +1     
==========================================
- Hits         5860     5852       -8     
- Misses       6313     6321       +8     
- Partials     1185     1186       +1     
Files Coverage Δ
kernel/engines/xuperos/config/config.go 68.00% <ø> (ø)
bcs/ledger/xledger/state/state.go 46.73% <0.00%> (-0.06%) ⬇️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@godeamon godeamon closed this Nov 23, 2023
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.

3 participants