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

FLIP 204: Include epoch timing config in sporking/bootstrapping process #5027

Conversation

jordanschalm
Copy link
Member

@jordanschalm jordanschalm commented Nov 16, 2023

Addresses #4947

  • Adds flags to configure epoch timing config to bootstrapping process, Localnet, and integration tests. These flags are set so that:
    • we can use the exact same values as in the smart contract config
    • in general, we won't need to change the flag values from spork to spork
  • Fixes a bug where execution state bootstrapping used the hard-coded flow.Genesis rather than the configured genesis block
  • Updates the pin of core-contracts to v0.15.0

yhassanzadeh13 and others added 30 commits November 14, 2023 13:27
This reverts commit 9b1a4cc.
[FVM] beyond EVM part 6.1 - Implement EVMAddress.deposit
[FVM] beyond EVM part 6.2 - Implement EVM.BridgedAccount.withdraw
…ow/flow-go into ramtin/evm-add-dummy-kitty-contract
[FVM] beyond EVM part 6.3 - Implement EVM.BridgedAccount.deploy
@jordanschalm
Copy link
Member Author

@durkmurder or @zhangchiqing Could I get another review pass on this PR? It has change significantly since Leo approved a few days ago.

cmd/bootstrap/cmd/seal.go Outdated Show resolved Hide resolved
cmd/bootstrap/cmd/finalize.go Outdated Show resolved Hide resolved
// Use Option 1 for Benchnet, Localnet, etc.
// Use Option 2 for Mainnet, Testnet, Canary.
finalizeCmd.Flags().BoolVar(&flagDefaultEpochTargetEndTime, "epoch-default-target-end-time", false, "whether to use the default target end time")
finalizeCmd.Flags().Uint64Var(&flagEpochTargetEndTimeRefCounter, "epoch-target-end-time-ref-counter", 0, "the reference epoch to use to compute the root epoch's target end time")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
finalizeCmd.Flags().Uint64Var(&flagEpochTargetEndTimeRefCounter, "epoch-target-end-time-ref-counter", 0, "the reference epoch to use to compute the root epoch's target end time")
finalizeCmd.Flags().Uint64Var(&flagRootEpochRefCounter, "root-epoch-ref-counter", 0, "the reference epoch to use to compute the root epoch's target end time")

Some suggestion to rename the variable. I found having the "TargetEndTime" in the name is a bit confusing. Having RootEpoch is enough.

Same to the following two flags if you agree this suggestion

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed it to epoch-timing-ref-counter instead, since the timing config conceptually defines the timing for all epochs. This also matches the naming of the smart contract config.

cmd/bootstrap/cmd/seal.go Outdated Show resolved Hide resolved
return fmt.Errorf("invalid epoch timing config: must specify ALL of --epoch-target-end-time-ref-counter, --epoch-target-end-time-ref-timestamp, and --epoch-target-end-time-duration")
}

// compute target end time for initial (root) epoch from flags: `TargetEndTime = now() + TargetDuration`
Copy link
Member

Choose a reason for hiding this comment

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

I don't get how is now() computed. Is it the same value for all nodes or different?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, this comment was outdated I think, from an earlier way I was thinking of computing this. Updated here: 996334c

Base automatically changed from jordan/4946-cruise-ctl-target-end-time to feature/flip-204-epoch-target-end-time November 24, 2023 20:32
@codecov-commenter
Copy link

codecov-commenter commented Nov 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (64356c8) 56.13% compared to head (64356c8) 56.13%.

❗ Current head 64356c8 differs from pull request most recent head 57f2065. Consider uploading reports for the commit 57f2065 to get more accurate results

Additional details and impacted files
@@                           Coverage Diff                           @@
##           feature/flip-204-epoch-target-end-time    #5027   +/-   ##
=======================================================================
  Coverage                                   56.13%   56.13%           
=======================================================================
  Files                                         944      944           
  Lines                                       87982    87982           
=======================================================================
  Hits                                        49389    49389           
  Misses                                      34895    34895           
  Partials                                     3698     3698           
Flag Coverage Δ
unittests 56.13% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This is just to get tests to pass, all these changes will be overwritten
with the update to version v0.15.0 in #5027
jordanschalm added a commit that referenced this pull request Nov 27, 2023
* Update cadence to v0.42.5

* use random string

* update godoc

* add in memory register store

* update comments

* add comment and update GetUpdatedRegister

* refactor IsErrPruned

* update tests

* prefix with tests InMemoryRegister

* use t.Run

* Apply suggestions from code review

Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>

* address review comments

* Apply suggestions from code review

Co-authored-by: Gregor G. <75445744+sideninja@users.noreply.github.com>

* fix linter

* fix linter

* fix lint

* use RegisterEntries

* add chainhash to integration

* add chainhash to insecure

* add chainhash to integration

* adds duration to the startup log

* update mocks

* adds TestNewSubscriptionRecordCache

* adds TestGetSubscribedTopics

* adds TestDuplicateTopics

* adds test move update cycle

* adds TestSubscriptionValidator_Integration

* adds TestMoveUpdateCycleWithDifferentPeers

* update emulator version

* updates mocks

* fixes build errors in insecure package

* adds TestSubscriptionProvider_GetSubscribedTopics_SkippingUnknownPeers

* minor fixes

* add comment explainin idProvider mock expectations

* use unittest random string

* add comment about continue statement

- remove obsolete continue statement

* Update fixtures.go

* small fixes

* fixes merge conflicts

* remove unnecessary variable declaration

* add support for testnet chain

* generalize support for all chains

* lint fix

* restoore gating EVM setup behind feature flag

* lint fix

* consolidates PeerIdFixtureB with PeerIdFixture

* adds lint and tidy to insecure package

* lint fix

* Revert "lint fix"

This reverts commit 9b1a4cc.

* lint fix

* revert the account not found error on withdraw

* define direct call types

* update docs

* remove unnecessary event types

* .

* add more contract types for testing

* fix tests

* skip test, requires funding (implemented in follow up PR)

* unskip test

* check error

* • extended compile-time checks for telemetry consumer enforcing that all happy-path interfaces are implemented
• extended subscriptions of telemetry consumer to receive missing events

* reduced notifications consumer interface in PaceMaker, as it only emits events from `hotstuff.ParticipantConsumer`

* update tests

* Refactor event emmision code

* Update network/p2p/unicast/README.MD

Co-authored-by: Khalil Claybon <khalil.claybon@dapperlabs.com>

* expose more test utility

* lint

* add register store

* update LastFinalizedAndExecutedHeight

* convert storage.ErrNotFound to nil

* fix lint

* fixes the logger

* moving ready to the select-case

* moving ready to select-case

* revises the error by update loop to make it irrecoverable

* clean up benchmarking

* adds select case for startup of subscription provider

* Update network/p2p/scoring/subscription_provider.go

Co-authored-by: Khalil Claybon <khalil.claybon@dapperlabs.com>

* adds documentation to subscription record cache

* fixes a bug with subscription record id

* adds more documentation for current cycle

* fix epoch event docs

* [Access] Handle script canceled and timeout errors

* [Access] Make script exec configurable

* Make computation limit configurable on ENs

* set limit setting once

* undo whitespace changes

* add storehouse loader

* add loaders

* add comments update log

* add comments

* update loader

* add extending block snapshot

* fix tests

* fix lint

* update interface

* update tests

* fix lint

* update committer

* update mocks

* update committer tests

* refactor collector

* update mocks

* update bootstrap

* fix noop committer

* fix computer_test

* fix tests

* refactor test

* update committer tests

* fix tests

* Update engine/access/rpc/backend/backend_scripts_test.go

Co-authored-by: Gregor G. <75445744+sideninja@users.noreply.github.com>

* fix committer tests

* add comment

* update tests

* Added implementation for execution api engine

* Added tests for execution api engine

* Fixed issues based on comments

* Added accidentally removed code

* Updated implementations following protobuf changes,updated mocks

* Linted

* [Access] Update websockets event streaming to return JSON-CDC encoded events

* Added tests for sentinel errors

* Update engine/execution/rpc/engine.go

Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>

* update NewStorageSnapshot

* update mocks

* update tests

* refactor script engine

* update mocks

* fix tests

* fix tests

* remove scripts engine tests

* update errors

* update tests

* refactor tests

* test CreateSnapshot

* add executing block snapshot

* reuse convert functions

* add block_end_snapshot tests

* remove changes

* Apply suggestions from code review

Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>

* add todo

* Update engine/execution/storehouse/block_end_snapshot_test.go

Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>

* Update engine/execution/storehouse/block_end_snapshot_test.go

Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>

* fix lint

* update comment

* add back interface

* update getFromStorage

* handle not found case

* add IsBlockExcuted

* update state commmitment by block id

* update mocks

* fix lint

* update unexecuted_loader

* fix ingestion tests

* fix stop control

* use IsParentExecuted

* fix loader tests

* Fix bug in reencoding and update tests

* update core-contracts version and state commitment constants

This is just to get tests to pass, all these changes will be overwritten
with the update to version v0.15.0 in #5027

---------

Co-authored-by: Yahya Hassanzadeh <yhassanzadeh13@ku.edu.tr>
Co-authored-by: Supun Setunga <supun.setunga@gmail.com>
Co-authored-by: Khalil Claybon <khalil.claybon@dapperlabs.com>
Co-authored-by: Gregor G <75445744+sideninja@users.noreply.github.com>
Co-authored-by: Misha <15269764+gomisha@users.noreply.github.com>
Co-authored-by: Leo Zhang (zhangchiqing) <zhangchiqing@gmail.com>
Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>
Co-authored-by: Bastian Müller <bastian@turbolent.com>
Co-authored-by: Gregor Gololicic <gregor.gololicic@dapperlabs.com>
Co-authored-by: Ramtin M. Seraj <ramtinms@users.noreply.github.com>
Co-authored-by: ramtinms <ramtin.seraj@dapperlabs.com>
Co-authored-by: Yahya Hassanzadeh, Ph.D <yhassanzadeh@ieee.org>
Co-authored-by: Khalil Claybon <kclaybon1@gmail.com>
Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>
Co-authored-by: Janez Podhostnik <67895329+janezpodhostnik@users.noreply.github.com>
Co-authored-by: Janez Podhostnik <janez.podhostnik@gmail.com>
Co-authored-by: Jan Bernatik <jan.bernatik@dapperlabs.com>
Co-authored-by: Andrii <andriy.dyachuk95@gmail.com>
Co-authored-by: Yurii Oleksyshyn <yuraolex@gmail.com>
Co-authored-by: Kan Zhang <kan@dapperlabs.com>
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.