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(cli): implement forest-cli f3 manifest #4937

Merged
merged 8 commits into from
Oct 28, 2024
Merged

feat(cli): implement forest-cli f3 manifest #4937

merged 8 commits into from
Oct 28, 2024

Conversation

hanabi1224
Copy link
Contributor

@hanabi1224 hanabi1224 commented Oct 28, 2024 •
edited
Loading

Summary of changes

This PR ports lotus f3 manifest CLI command.

Changes introduced in this pull request:

  • implement forest-cli f3 manifest command

Output:

➜  forest git:(hm/forest-cli-f3-manifest) cargo run --bin forest-cli -- f3 manifest --output text
   Compiling forest-filecoin v0.21.1 (/home/me/git/forest)
    Finished `dev` profile [unoptimized] target(s) in 22.60s
     Running `target/debug/forest-cli f3 manifest --output text`
Manifest:
  Protocol Version:     4
  Paused:               false
  Initial Instance:     0
  Initial Power Table:  bafy2bzaceab236vmmb3n4q4tkvua2n4dphcbzzxerxuey3mot4g3cov5j3r2c
  Bootstrap Epoch:      2081674
  Network Name:         calibrationnet
  Ignore EC Power:      false
  Committee Lookback:   10
  Catch Up Alignment:   15s

  GPBFT Delta:                        6s
  GPBFT Delta BackOff Exponent:       2.0
  GPBFT Max Lookahead Rounds:         5
  GPBFT Rebroadcast Backoff Base:     6s
  GPBFT Rebroadcast Backoff Exponent: 1.3
  GPBFT Rebroadcast Backoff Spread:   0.1
  GPBFT Rebroadcast Backoff Max:      1m

  EC Period:            30s
  EC Finality:          900
  EC Delay Multiplier:  2.0
  EC Head Lookback:     0
  EC Finalize:          true

  Certificate Exchange Client Timeout:    10s
  Certificate Exchange Server Timeout:    1m
  Certificate Exchange Min Poll Interval: 30s
  Certificate Exchange Max Poll Interval: 2m
➜  forest git:(hm/forest-cli-f3-manifest) cargo run --bin forest-cli -- f3 manifest --output json
   Compiling forest-filecoin v0.21.1 (/home/me/git/forest)
    Finished `dev` profile [unoptimized] target(s) in 14.69s
     Running `target/debug/forest-cli f3 manifest --output json`
{
  "ProtocolVersion": 4,
  "Pause": false,
  "InitialInstance": 0,
  "BootstrapEpoch": 2081674,
  "NetworkName": "calibrationnet",
  "ExplicitPower": null,
  "IgnoreECPower": false,
  "InitialPowerTable": {
    "/": "bafy2bzaceab236vmmb3n4q4tkvua2n4dphcbzzxerxuey3mot4g3cov5j3r2c"
  },
  "CommitteeLookback": 10,
  "CatchUpAlignment": 15000000000,
  "Gpbft": {
    "Delta": 6000000000,
    "DeltaBackOffExponent": 2.0,
    "MaxLookaheadRounds": 5,
    "RebroadcastBackoffBase": 6000000000,
    "RebroadcastBackoffExponent": 1.3,
    "RebroadcastBackoffSpread": 0.1,
    "RebroadcastBackoffMax": 60000000000
  },
  "EC": {
    "Period": 30000000000,
    "Finality": 900,
    "DelayMultiplier": 2.0,
    "BaseDecisionBackoffTable": [
      1.3,
      1.69,
      2.2,
      2.86,
      3.71,
      4.83,
      6.27,
      7.5
    ],
    "HeadLookback": 0,
    "Finalize": true
  },
  "CertificateExchange": {
    "ClientRequestTimeout": 10000000000,
    "ServerRequestTimeout": 60000000000,
    "MinimumPollInterval": 30000000000,
    "MaximumPollInterval": 120000000000
  }
}

Reference issue to close (if applicable)

Closes #4932

Other information and links

Change checklist

  • I have performed a self-review of my own code,
  • I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
  • I have added tests that prove my fix is effective or that my feature works (if possible),
  • I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

@hanabi1224 hanabi1224 marked this pull request as ready for review October 28, 2024 09:01
@hanabi1224 hanabi1224 requested a review from a team as a code owner October 28, 2024 09:01
@hanabi1224 hanabi1224 requested review from lemmih and LesnyRumcajs and removed request for a team October 28, 2024 09:01
@hanabi1224 hanabi1224 mentioned this pull request Oct 28, 2024
Merged
8 tasks
hanabi1224
hanabi1224 commented Oct 28, 2024
View reviewed changes
src/rpc/methods/f3/types.rs
#[serde_as]
#[derive(PartialEq, Debug, Clone, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "PascalCase")]
pub struct F3Manifest {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ruseinov These structs should be moved into rust-f3 eventually

hanabi1224
hanabi1224 commented Oct 28, 2024
View reviewed changes
templates/cli/f3/manifest.stpl
@@ -0,0 +1,29 @@
Manifest:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Format comes from https://github.com/filecoin-project/lotus/blob/master/cli/templates/f3_manifest.go.tmpl

LesnyRumcajs
LesnyRumcajs reviewed Oct 28, 2024
View reviewed changes
Cargo.toml Outdated
@@ -48,6 +48,7 @@ directories = "5"
displaydoc = "0.2"
ethereum-types = "0.15"
ez-jsonrpc-types = "0.3"
fancy-duration = "0.9"
Copy link
Member

Choose a reason for hiding this comment

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

It seems like a pretty niche dependency without many users - do you think it makes sense depending on it or can we avoid? https://crates.io/crates/fancy-duration/reverse_dependencies

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me see if I can find a better replacement. I first spotted https://crates.io/crates/human-duration but found it's not been updated for 3 years

Copy link
Contributor Author

@hanabi1224 hanabi1224 Oct 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@LesnyRumcajs fancy-duration is the best I can find under https://crates.io/search?q=duration

chrono::Duration displays 15s as PT15S which follows https://en.wikipedia.org/wiki/ISO_8601#Durations but is not super readable for non-tech users

Copy link
Member

Choose a reason for hiding this comment

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

What about humantime? It's a bit of an abandonware, but we already have it in our dependencies.

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.

@hanabi1224 hanabi1224 enabled auto-merge October 28, 2024 13:46
@hanabi1224
Copy link
Contributor Author

@LesnyRumcajs resolved a conflict in CHANGELOG, please re-approve, thanks!

@hanabi1224 hanabi1224 added this pull request to the merge queue Oct 28, 2024
Merged via the queue into main with commit 454db3b Oct 28, 2024
35 checks passed
@hanabi1224 hanabi1224 deleted the hm/forest-cli-f3-manifest branch October 28, 2024 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LesnyRumcajs LesnyRumcajs LesnyRumcajs approved these changes

@lemmih lemmih lemmih approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[F3] port lotus f3 manifest CLI command
3 participants
@hanabi1224 @lemmih @LesnyRumcajs