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.

Sign up for GitHub

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

Jump to bottom

feat: drand quicknet: allow scheduling drand quicknet upgrade before nv22 on 2k devnet #11632

Merged
merged 5 commits into from
Feb 28, 2024
Merged

feat: drand quicknet: allow scheduling drand quicknet upgrade before nv22 on 2k devnet #11632

merged 5 commits into from
Feb 28, 2024

Conversation

hanabi1224
Copy link
Contributor

@hanabi1224 hanabi1224 commented Feb 20, 2024 •
edited
Loading

Related Issues

Currently, there's code using the blockchain network version to determine using chained or unchained drand beacon verification. This PR tries to decouple the logic so that on 2k devnet the drand quicknet upgrade can be scheduled before the Dragon upgrade, to facilitate testing on forest side. The change has been verified by the forest devnet tests in ChainSafe/forest#3969

Proposed Changes

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • If the PR affects users (e.g., new feature, bug fix, system requirements change), update the CHANGELOG.md and add details to the UNRELEASED section.
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

@hanabi1224 hanabi1224 requested a review from a team as a code owner February 20, 2024 12:28
@hanabi1224 hanabi1224 requested review from Stebalien and magik6k and removed request for a team February 20, 2024 12:28
@hanabi1224 hanabi1224 changed the title feat: allow scheduling drand quicknet upgrade before nv22 on 2k devnet feat: drand quicknet: allow scheduling drand quicknet upgrade before nv22 on 2k devnet Feb 20, 2024
arajasek
arajasek reviewed Feb 20, 2024
View reviewed changes
Copy link
Contributor

@arajasek arajasek left a comment

Choose a reason for hiding this comment

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

This looks generally good to me, thanks a lot for the contribution! I have one suggested improvement, but otherwise this is landable.

chain/beacon/drand/drand.go Outdated
@@ -37,7 +37,8 @@ var log = logging.Logger("drand")
//
// The root trust for the Drand chain is configured from build.DrandChain.
type DrandBeacon struct {
client dclient.Client
network dtypes.DrandEnum
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we instead make this a simple boolean chained, and set that at config time? I think that will simplify this overall, and also avoid one-offs like db.network != build.DrandQuicknet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@jennijuju jennijuju added this to the Network v22 milestone Feb 21, 2024
@jennijuju jennijuju added the P2 P2: Should be resolved label Feb 21, 2024
arajasek
arajasek approved these changes Feb 23, 2024
View reviewed changes
Copy link
Contributor

@arajasek arajasek left a comment

Choose a reason for hiding this comment

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

Approved with one nit.

chain/beacon/beacon.go Outdated
if nv < network.Version22 {
// When we have "chained" beacons, two entries at a fork are required.
currBeacon := bSchedule.BeaconForEpoch(epoch)
if currBeacon.IsChained() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we simplify this to:

		parentBeacon := bSchedule.BeaconForEpoch(parentEpoch)
		currBeacon := bSchedule.BeaconForEpoch(epoch)
		if parentBeacon != currBeacon && currBeacon.IsChained() {
// fork logic
}

Then we can also drop line 140 (and just use currBeacon everywhere).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@arajasek arajasek merged commit 6952a8d into filecoin-project:feat/nv22 Feb 28, 2024
90 checks passed
@hanabi1224 hanabi1224 deleted the hm/drand-test branch February 29, 2024 02:28
@hanabi1224 hanabi1224 mentioned this pull request Feb 29, 2024
Merged
4 tasks
rjan90 pushed a commit that referenced this pull request Mar 1, 2024
@arajasek @rjan90
Merge pull request #11632 from hanabi1224/hm/drand-test
f2bca58 
feat: drand quicknet: allow scheduling drand quicknet upgrade before nv22 on 2k devnet
@rjan90 rjan90 mentioned this pull request Mar 1, 2024
Merged
8 tasks
rjan90 added a commit that referenced this pull request Mar 1, 2024
@rjan90
Merge pull request #11667 from filecoin-project/phi-backport-11632
62989ff 
chore: backport #11632 to release/v1.26.0
rjan90 pushed a commit that referenced this pull request Mar 6, 2024
@arajasek @rjan90
Merge pull request #11632 from hanabi1224/hm/drand-test
7176dc3 
feat: drand quicknet: allow scheduling drand quicknet upgrade before nv22 on 2k devnet
rjan90 added a commit that referenced this pull request Mar 12, 2024
@rjan90 @arajasek
@magik6k @jennijuju @snadrus @Stebalien @rvagg @samuelarogbonlo @LexLuthr @aarshkshah1992 @masih
chore: Merge nv22 into master (#11699)
6f7498b 
* [WIP] feat: Add nv22 skeleton

Addition of Network Version 22 skeleton

* update FFI

* feat: drand: refactor round verification

* feat: sealing: Support nv22 DDO features in the sealing pipeline (#11226)

* Initial work supporting DDO pieces in lotus-miner

* sealing: Update pipeline input to operate on UniversalPiece

* sealing: Update pipeline checks/sealing states to operate on UniversalPiece

* sealing: Make pipeline build with UniversalPiece

* move PieceDealInfo out of api

* make gen

* make sealing pipeline unit tests pass

* fix itest ensemble build

* don't panic in SectorsStatus with deals

* stop linter from complaining about checkPieces

* fix sector import tests

* mod tidy

* sealing: Add logic for (pre)committing DDO sectors

* sealing: state-types with method defs

* DDO non-snap pipeline works(?), DDO Itests

* DDO support in snapdeals pipeline

* make gen

* update actor bundles

* update the gst market fix

* fix: chain: use PreCommitSectorsBatch2 when setting up genesis

* some bug fixes

* integration working changes

* update actor bundles

* Make TestOnboardRawPieceSnap pass

* Appease the linter

* Make deadlines test pass with v12 actors

* Update go-state-types, abstract market DealState

* make gen

* mod tidy, lint fixes

* Fix some more tests

* Bump version in master

Bump version in master

* Make gen

Make gen

* fix sender

* fix: lotus-provider: Fix winning PoSt

* fix: sql Scan cannot write to an object

* Actually show miner-addrs in info-log

Actually show miner-addrs in lotus-provider info-log

* [WIP] feat: Add nv22 skeleton

Addition of Network Version 22 skeleton

* update FFI

* ddo is now nv22

* make gen

* temp actor bundle with ddo

* use working go-state-types

* gst with v13 market migration

* update bundle, builtin.MethodsMiner.ProveCommitSectors2 -> 3

* actually working v13 migration, v13 migration itest

* Address review

* sealing: Correct DDO snap pledge math

* itests: Mixed ddo itest

* pipeline: Fix sectorWeight

* sealing: convert market deals into PAMs in mixed sectors

* sealing: make market to ddo conversion work

* fix lint

* update gst

* Update actors and GST to lastest integ branch

* commit batcher: Update ProveCommitSectors3Params builder logic

* make gen

* use builtin-actors master

* ddo: address review

* itests: Add commd assertions to ddo tests

* make gen

* gst with fixed types

* config knobs for RequireActivationSuccess

* storage: Drop obsolete flaky tasts

---------

Co-authored-by: Jennifer Wang <[email protected]>
Co-authored-by: Aayush <[email protected]>
Co-authored-by: Shrenuj Bansal <[email protected]>
Co-authored-by: Phi <[email protected]>
Co-authored-by: Andrew Jackson (Ajax) <[email protected]>
Co-authored-by: TippyFlits <[email protected]>

* feat: implement FIP-0063

* chore: deps: update to go-multiaddr v0.12.2 (#11602)

* feat: fvm: update the FVM/FFI to v4.1 (#11608) (#11612)

This:

1. Adds nv22 support.
2. Updates the message tracing format.

Co-authored-by: Steven Allen <[email protected]>

* AggregateProofType nil when doing batch updates

Use latest nv22 go-state-types version with matching update

* Update to v13.0.0-rc.2 bundle

* chore: Upgrade heights and codename

Update upgrade heights

Co-Authored-By: Steven Allen <[email protected]>

* Update epoch after nv22 DRAND switch

Update epoch after nv22 DRAND switch

* Update Mango codename to Phoneix

Make the codename for the Drand-change inline with Dragon style.

* Add UpgradePhoenixHeight to API params

* set UpgradePhoenixHeight to be one hour after Dragon

* Make gen

Make gen and UpgradePhoenixHeight in butterfly and local devnet to be in line with Calibration and Mainnet

* Update epoch heights (#11637)

Update epoch heights

* new: add forest bootstrap nodes (#11636)

Signed-off-by: samuelarogbonlo <[email protected]>

* Merge pull request #11491 from filecoin-project/fix/remove-decommissioned-pl-bootstrap-nodes

Remove PL operated bootstrap nodes from mainnet.pi

* feat: api: new verified registry methods to get all allocations and claims (#11631)

* new verireg methods

* update changelog and add itest

* update itest and cli

* update new method's support till v9

* remove gateway APIs

* fix cli internal var names

* chore:: backport #11609 to the feat/nv22 branch (#11644)

* feat: api: improve the correctness of Eth's trace_block (#11609)

* Improve the correctness of Eth's trace_block

- Improve encoding/decoding of parameters and return values:
  - Encode "native" parameters and return values with Solidity ABI.
  - Correctly decode parameters to "create" calls.
  - Use the correct (ish) output for "create" calls.
  - Handle all forms of "create".
- Make robust with respect to reverts:
  - Use the actor ID/address from the trace instead of looking it up in
    the state-tree (may not exist in the state-tree due to a revert).
  - Gracefully handle failed actor/contract creation.
- Improve performance:
  - We avoid looking anything up in the state-tree when translating the
    trace, which should significantly improve performance.
- Improve code readability:
  - Remove all "backtracking" logic.
  - Use an "environment" struct to store temporary state instead of
    attaching it to the trace.
- Fix random bugs:
  - Fix an allocation bug in the "address" logic (need to set the
    capacity before modifying the slice).
  - Improved error checking/handling.
- Use correct types for `trace_block` action/results (create, call, etc.).
  - And use the correct types for Result/Action structs instead of reusing the same "Call" action every time.
- Improve error messages.

* Make gen

Make gen

---------

Co-authored-by: Steven Allen <[email protected]>

* fix: add UpgradePhoenixHeight to StateGetNetworkParams (#11648)

* chore: deps: update to go-state-types v13.0.0-rc.1

* do NOT update the cache when running the real migration

* Merge pull request #11632 from hanabi1224/hm/drand-test

feat: drand quicknet: allow scheduling drand quicknet upgrade before nv22 on 2k devnet

* chore: deps: update to go-state-types v13.0.0-rc.2

chore: deps: update to go-state-types v13.0.0-rc.2

* feat: set migration config UpgradeEpoch for v13 actors upgrade

* Built-in actor events first draft

* itest for DDO non-market verified data w/ builtin actor events

* Tests for builtin actor events API

* Clean up DDO+Events tests, add lots of explainer comments

* Minor tweaks to events types

* Avoid duplicate messages when looking for receipts

* Rename internal events modules for clarity

* Adjust actor event API after review

* s/ActorEvents/Events/g in global config

* Manage event sending rate for SubscribeActorEvents

* Terminate SubscribeActorEvents chan when at max height

* Document future API changes

* More clarity in actor event API docs

* More post-review changes, lots of tests for SubscribeActorEvents

Use BlockDelay as the window for receiving events on the SubscribeActorEvents
channel. We expect the user to have received the initial batch of historical
events (if any) in one block's time. For real-time events we expect them to
not fall behind by roughly one block's time.

* Remove duplicate code from actor event type marshalling tests

Reduce verbosity and remove duplicate test logic from actor event types
JSON marshalling tests.

* Rename actor events test to follow go convention

Add missing `s` to `actor_events` test file to follow golang convention
used across the repo.

* Run actor events table tests in deterministic order

Refactor `map` usage for actor event table tests to ensure deterministic
test execution order, making debugging potential issues easier. If
non-determinism is a target, leverage Go's built-in parallel testing
capabilities.

* Reduce scope for filter removal failure when getting actor events

Use a fresh context to remove the temporary filter installed solely to
get the actor events. This should reduce chances of failure in a case
where the original context may be expired/cancelled.

Refactor removal into a `defer` statement for a more readable, concise
return statement.

* Use fixed RNG seed for actor event tests

Improve determinism in actor event tests by using a fixed RNG seed. This
makes up a more reproducible test suit.

* Use provided libraries to assert eventual conditions

Use the functionalities already provided by `testify` to assert eventual
conditions, and remove the use of `time.Sleep`.

Remove duplicate code in utility functions that are already defined.

Refactor assertion helper functions to use consistent terminology:
"require" implies fatal error, whereas "assert" implies error where the
test may proceed executing.

* Update changelog for actor events APIs

* Fix concerns and docs identified by review

* Update actor bundle to v13.0.0-rc3

Update actor bundle to v13.0.0-rc3

* Prep Lotus v1.26.0-rc1

- For sanity reverting the mainnet upgrade epoch to 99999999, and then only set it when cutting the final release

-Update Calibnet CIDs to v13.0.0-rc3

- Add GetActorEvents, SubscribeActorEvents, GetAllClaims and GetAllAllocations methods to the changelog

Co-Authored-By: Jiaying Wang <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Masih H. Derkani <[email protected]>

* Make gen

Make gen

* fix: beacon: validate drand change at nv16 correctly

* bump to v1.26.0-rc2

* test: cleanup ddo verified itest, extract steps to functions

also add allocation-removed event case

* test: extract verified DDO test to separate file, add more checks

* test: add additional actor events checks

* Add verification for "deal-activated" actor event

* docs(drand): document the meaning of "IsChained" (#11692)

* Resolve conflicts

I encountered multiple issues when trying to run make gen. And these changes fixed a couple of them:
- go mod tidy
- Remove RaftState/RaftLeader
- Revert `if ts.Height() > claim.TermMax+claim.TermStart || !cctx.IsSet("expired")` to the what is in the release/v1.26.0: `if tsHeight > val.TermMax || !expired`

* fixup imports, make jen

* Update version

Update version in master to v1.27.0-dev

* Update node/impl/full/dummy.go

Co-authored-by: Łukasz Magiera <[email protected]>

* Adjust ListClaimsCmd

Adjust ListClaimsCmd according to review

---------

Signed-off-by: samuelarogbonlo <[email protected]>
Co-authored-by: TippyFlits <[email protected]>
Co-authored-by: Aayush <[email protected]>
Co-authored-by: Łukasz Magiera <[email protected]>
Co-authored-by: Jennifer Wang <[email protected]>
Co-authored-by: Shrenuj Bansal <[email protected]>
Co-authored-by: Andrew Jackson (Ajax) <[email protected]>
Co-authored-by: Steven Allen <[email protected]>
Co-authored-by: Rod Vagg <[email protected]>
Co-authored-by: Samuel Arogbonlo <[email protected]>
Co-authored-by: LexLuthr <[email protected]>
Co-authored-by: tom123222 <[email protected]>
Co-authored-by: Aarsh Shah <[email protected]>
Co-authored-by: Masih H. Derkani <[email protected]>
Co-authored-by: Jiaying Wang <[email protected]>
rjan90 pushed a commit that referenced this pull request Mar 22, 2024
@arajasek @rjan90
Merge pull request #11632 from hanabi1224/hm/drand-test
10aeace 
feat: drand quicknet: allow scheduling drand quicknet upgrade before nv22 on 2k devnet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arajasek arajasek arajasek approved these changes

@Stebalien Stebalien Awaiting requested review from Stebalien Stebalien is a code owner automatically assigned from filecoin-project/lotus-maintainers

@magik6k magik6k Awaiting requested review from magik6k magik6k is a code owner automatically assigned from filecoin-project/lotus-maintainers

Assignees
No one assigned
Labels
P2 P2: Should be resolved
Projects
FilOz
Status: ☑️ Done (Archive)
Milestone
Network v22
Development

Successfully merging this pull request may close these issues.
3 participants
@hanabi1224 @arajasek @jennijuju