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

Inconsistency in Istanbul.LookbackWindow validation #1687

Open
piersy opened this issue Sep 14, 2021 · 0 comments
Open

Inconsistency in Istanbul.LookbackWindow validation #1687

piersy opened this issue Sep 14, 2021 · 0 comments

Comments

@piersy
Copy link
Contributor

piersy commented Sep 14, 2021

Description

The celo-blockchain has the following validation

if chainConfig.Istanbul.LookbackWindow >= chainConfig.Istanbul.Epoch-2 {
return fmt.Errorf("istanbul.lookbackwindow must be less than istanbul.epoch-2")
}

And celo-monorepo has the following.

    require(
      window <= getEpochSize().sub(2),
      "UptimeLookbackWindow must be smaller or equal to epochSize - 2"
    );

https://github.com/celo-org/celo-monorepo/blob/64e618b9b856073305dd5748fc04fc772ff72714/packages/protocol/contracts/governance/BlockchainParameters.sol#L110-L113

Proposed Solution

I'm not sure which is correct, but I'm thinking maybe the validation should be removed from the blockchain, since validation seems to be done at the contract level making the blockchain validation redundant and the error returned from the contract seems quite readable.

I noticed also that this parameter is set in 2 places in the config.

LookbackWindow uint64 `json:"lookbackwindow"` // The number of blocks to look back when calculating uptime

and

UptimeLookbackWindow uint64 `json:"uptimeLookbackWindow"`

Revert: UptimeLookbackWindow must be smaller or equal to epochSize - 2
@carterqw2 carterqw2 added triage Issue needs triaging and removed blockchain labels Feb 9, 2023
@carterqw2 carterqw2 added good first issue and removed triage Issue needs triaging labels Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants