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

Author a sibling block in case best block's slot is same as current slot #2726

Merged
merged 14 commits into from
Sep 9, 2022

Conversation

kishansagathiya
Copy link
Contributor

Changes

  • While authoring a block in a given slot S, it could happen that best block was also authored in the slot S. In this case, we can't create a new block as a child of best block, because then we would have a chain where slot numbers don't increase.
  • In such scenario, we should author a sibling block to best block. That is we should author a block as a child of best block's parent.
  • After authoring such a block, we would have two blocks with same parent, same block number and same slot. This would create a fork, but that would eventually get resolved.

Tests

go test -tags integration github.com/ChainSafe/gossamer

Issues

Fixes #2636

Primary Reviewer

@timwu20

the current slot, we should author a new block as child of best block's
parent.

This would create forks in the chain which would get resolved
eventually.
lib/babe/babe.go Outdated Show resolved Hide resolved
lib/babe/babe.go Outdated Show resolved Hide resolved
lib/babe/babe.go Outdated Show resolved Hide resolved
lib/babe/babe.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Aug 4, 2022

Codecov Report

Merging #2726 (d022f43) into development (f635c59) will increase coverage by 0.13%.
The diff coverage is 0.00%.

Additional details and impacted files
@@               Coverage Diff               @@
##           development    #2726      +/-   ##
===============================================
+ Coverage        62.88%   63.01%   +0.13%     
===============================================
  Files              213      213              
  Lines            27021    27097      +76     
===============================================
+ Hits             16992    17076      +84     
+ Misses            8476     8464      -12     
- Partials          1553     1557       +4     

lib/babe/babe.go Outdated Show resolved Hide resolved
Copy link
Member

@EclesioMeloJunior EclesioMeloJunior left a comment

Choose a reason for hiding this comment

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

LGTM! Is it possible to check in the import block routine if the block we are currently importing contains a greater slot than our next authoring slot? For example, if gossamer imports block X with contains slot 10 but our next authoring slot is 8 so X should wait until we reach that or a great slot

lib/babe/babe.go Show resolved Hide resolved
lib/babe/babe.go Outdated Show resolved Hide resolved
@kishansagathiya
Copy link
Contributor Author

if gossamer imports block X with contains slot 10 but our next authoring slot is 8 so X should wait until we reach that or a great slot

@EclesioMeloJunior, what do you mean when you say X should wait? Do you mean we should not import block X?
Your described scenario could happen because of two reason:

  • We are lagging behind or
  • X is an invalid block

I think in this scenario, we should just author blocks regularly. We author for slot 8 even though we received a block with slot 10. It is problematic only if in a chain (without forks, so not a tree), slots of all block should be increasing. You could author a block for slot 8 and create a fork.

@kishansagathiya
Copy link
Contributor Author

I was facing problems with tests. Seeing a lot of

2022-08-23T13:58:29+05:30 WARN failed to apply extrinsic 0xad018400d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d01e08635fff8fd7b307827575dbd8f17a96ad9de6c15e690806d8c22ef410ae2668ac8f88e697f73414a20fd905556860688a9e8b661c819f2ee00774b5023d082000000000108abcd: transaction validity error: bad proof	pkg=babe
2022-08-23T13:58:31+05:30 CRIT target=runtime message=panicked at 'Timestamp must be updated once in the block', /home/elizabeth/substrate/frame/timestamp/src/lib.rs:170:13	ext_logging_log_version_1	pkg=runtime module=go-wasmer
2022-08-23T13:58:31+05:30 WARN failed to handle slot 553747769: cannot finalise block: running runtime function: Failed to call the `BlockBuilder_finalize_block` exported function.	pkg=babe

noot mentioned that this could be because of a race condition. Below logs might help

https://gist.github.com/kishansagathiya/d64d70dc48bbde60c7ceb3813a4a266c

Key crypto.PublicKey
Key crypto.PublicKey
// Weight exists for potential improvements in the protocol and could
// have a use-case in the future. In polkadot all authorities have the weight = 1.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

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.

Import failed: Slot number must increase
4 participants