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

Feature: 1 second finality #4771

Open
wants to merge 26 commits into
base: dev
Choose a base branch
from
Open

Feature: 1 second finality #4771

wants to merge 26 commits into from

Conversation

Frozen
Copy link
Contributor

@Frozen Frozen commented Oct 13, 2024

added 2 params.Config
	IsOneSecondEpoch *big.Int `json:"is-one-second-epoch,omitempty"`

	IsRotationEachBlockEpoch *big.Int `json:"is-rotation-each-block-epoch"`
applied with
LeaderRotationInternalValidatorsEpoch: big.NewInt(2),
LeaderRotationExternalValidatorsEpoch: big.NewInt(2),
IsOneSecondEpoch:         big.NewInt(2),
IsRotationEachBlockEpoch: big.NewInt(2),
commission calculation
	// TwoSecStakedBlocks is the flat-rate block reward after epoch 360.
	// 7 ONE per block
	TwoSecStakedBlocks = numeric.NewDecFromBigInt(new(big.Int).Mul(
		big.NewInt(7*denominations.Nano), big.NewInt(denominations.Nano),
	))
	// HIP30StakedBlocks is the reward received after HIP-30 goes into
	// effect. It is simply double the TwoSecStakedBlocks reward, since
	// the number of shards is being halved and we keep emission
	// constant.
	HIP30StakedBlocks = numeric.NewDecFromBigInt(new(big.Int).Mul(
		big.NewInt(14*denominations.Nano), big.NewInt(denominations.Nano),
	))

	// OneSecStakedBlock is half of HIP30
	OneSecStakedBlock = numeric.NewDecFromBigInt(new(big.Int).Mul(
		big.NewInt(7*denominations.Nano), big.NewInt(denominations.Nano),
	))

1-second epoch finality.

Copy link

mergify bot commented Oct 13, 2024

⚠️ The sha of the head commit of this PR conflicts with #4738. Mergify cannot evaluate rules on this PR. ⚠️

@Frozen Frozen mentioned this pull request Oct 13, 2024
consensus.GetLogger().Info().Str("waitTime", waitTime.String()).
Msg("[OnCommit] Starting Grace Period")
time.Sleep(waitTime)

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using a hard-coded delay of 200 ms, Maybe we can think of implementing a timer with a configurable timeout. This way, the process can use every millisecond effectively. If additional time becomes available, the timer can wait just the necessary amount, ensuring a more efficient use of time and avoiding unnecessary delays.

func (c *ChainConfig) IsRotationEachBlock(epoch *big.Int) bool {
return isForked(c.IsRotationEachBlockEpoch, epoch)
}

// UpdateEthChainIDByShard update the ethChainID based on shard ID.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is RotationEachBlockEpoch ??? do you mean "Leader Rotation at Epoch Block"?

Copy link
Contributor

Choose a reason for hiding this comment

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

same question here

@@ -724,6 +741,21 @@ func (consensus *Consensus) rotateLeader(epoch *big.Int, defaultKey *bls.PublicK
utils.Logger().Error().Err(err).Msg("Failed to find committee")
return defaultKey
}

if bc.Config().IsRotationEachBlock(epoch) {
Copy link
Contributor

@sophoah sophoah Oct 23, 2024

Choose a reason for hiding this comment

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

why are we doing that now ? or is this a new config rename that was meant for HIP32 ?

@sophoah
Copy link
Contributor

sophoah commented Oct 23, 2024

i also tested in localnet the PR, there are issue with the signature collection as soon as the 1s epoch start (currently set at epoch 6)

        {
          "blocks": {
            "signed": 89,
            "to-sign": 128
          },
          "epoch": 7
        },
        {
          "blocks": {
            "signed": 87,
            "to-sign": 128
          },
          "epoch": 6
        },
        {
          "blocks": {
            "signed": 122,
            "to-sign": 128
          },
          "epoch": 5
        },
        {
          "blocks": {
            "signed": 128,
            "to-sign": 128
          },
          "epoch": 4
        },

We can see the same issue in the node logs as numSignatures field is sometime set to 0
image

@Frozen Frozen force-pushed the feature/1-second-finality branch from 2659d6b to dd4b159 Compare November 6, 2024 19:34
@Frozen Frozen force-pushed the feature/1-second-finality branch from dd4b159 to c9518a5 Compare December 18, 2024 17:55
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