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

chore: f3: Upgrade to latest go-f3 v0.0.4 #12237

Merged
merged 1 commit into from
Jul 15, 2024
Merged

chore: f3: Upgrade to latest go-f3 v0.0.4 #12237

merged 1 commit into from
Jul 15, 2024

Conversation

masih
Copy link
Member

@masih masih commented Jul 15, 2024

Related Issues

#12236

Proposed Changes

Upgrade to the latest go F3, with fix to default cert exchange config in local devnet.

Additional Info

None

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, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • Update CHANGELOG.md or signal that this change does not need it.
    • 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.
    • If the change does not require a CHANGELOG.md entry, do one of the following:
      • Add [skip changelog] to the PR title
      • Add the label skip/changelog to the PR
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

@masih masih changed the title Upgrade to latest go-f3 v0.0.4 chore: f3: Upgrade to latest go-f3 v0.0.4 Jul 15, 2024
@masih masih force-pushed the masih/dep-up-f3 branch from a647612 to da15f59 Compare July 15, 2024 13:07
@masih masih marked this pull request as draft July 15, 2024 13:22
Upgrade to the latest go F3, with fix to default cert exchange config in
local devnet.
@masih masih force-pushed the masih/dep-up-f3 branch from da15f59 to 71f8ecd Compare July 15, 2024 13:30
@masih masih requested review from Kubuxu and Stebalien July 15, 2024 13:30
@masih masih marked this pull request as ready for review July 15, 2024 13:30
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

Note: this also changes the manifest protocol. But we haven't deployed that server yet so we should be fine.

@Stebalien Stebalien merged commit 7cc8db8 into master Jul 15, 2024
79 checks passed
@Stebalien Stebalien deleted the masih/dep-up-f3 branch July 15, 2024 14:46
"github.com/filecoin-project/lotus/node/modules/dtypes"
)

func NewManifestProvider(nn dtypes.NetworkName, cs *store.ChainStore, sm *stmgr.StateManager, ps *pubsub.PubSub) manifest.ManifestProvider {
func NewManifestProvider(nn dtypes.NetworkName, ps *pubsub.PubSub) manifest.ManifestProvider {
m := manifest.LocalDevnetManifest()
Copy link
Member

Choose a reason for hiding this comment

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

@masih both MinimumPollInterval and MaximumPollInterval are based on DefaultEcConfig.ECPeriod in go-f3 but we don't reset them here. Should we? I believe they come out the same here as long as BlockDelaySecs is 30. How much do these matter to align to epoch length?

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 believe they come out the same here as long as BlockDelaySecs is 30

I think regardless of what BlockDelaySecs is set to cert exchange min max would be 30s and 2m respectively, because their value is instantiated via DefaultCxConfig var using DefaultEcConfig.ECPeriod.

For example, this passes for me.

import (
	"testing"
	"time"

	"github.com/filecoin-project/go-f3/manifest"
)

func TestMinMaxPollIntervalIsNotOverridden(t *testing.T) {
	m := manifest.LocalDevnetManifest()
	m.ECPeriod = 1
	if m.CxConfig.MinimumPollInterval != 30*time.Second {
		t.Fatalf("min ain't it")
	}
	if m.CxConfig.MaximumPollInterval != 4*30*time.Second {
		t.Fatalf("max ain't it")
	}
}

How much do these matter to align to epoch length?

Cert exchange internally optimises for the right poll time. As long as min and max are a wide enough interval of time, and min is not less than an epoch we should be good.

@Stebalien does that sound right?

@rjan90 rjan90 mentioned this pull request Jul 22, 2024
8 tasks
rjan90 pushed a commit that referenced this pull request Jul 22, 2024
Upgrade to the latest go F3, with fix to default cert exchange config in
local devnet.
rjan90 added a commit that referenced this pull request Jul 24, 2024
* Update changelog

Update changelog

* chore: lotus-shed: Add support for nv23 in `migrate-state` cmd (#12172)

* Add support for nv23 in `lotus-shed migrate-state` cmd

Add support for nv23 in `lotus-shed migrate-state` cmd

* Add actors v14 to `check-invariants` cmd

Add actors v14 to `check-invariants` cmd

* feat: F3: Update go-f3, change the style of participation call (#12196)

* Upgrade go mod F3 dependency to v0.0.3 tagged release

Use concrete/latest version of F3: v0.0.3

* update changelog

* chore!: markets: remove deal constants

* Upgrade to OpenTelemetry v1.28.0

Upgrade to new version of OpenTelemetry to also play nice with F3.

* Reduce log level in F3 message sending to Debug (#12224)

Reduce the log level for `miner with id...` to DEBUG, since in normal
operation of F3 at least 5 messages are sent per epoch. In an event of
rebroadcast the number of messages broadcasted could surpass 10. Hence,
the `DEBUG` log level.

* chore: deps: upgrade to [email protected]

* Upgrade to latest go-f3 `v0.0.4` (#12237)

Upgrade to the latest go F3, with fix to default cert exchange config in
local devnet.

* chore: config: yet more lp2p removal from miner

* fix(store): correctly break weight ties based on smaller ticket (#12253)

@rjan90 and @jennijuju reported seeing this log a lot, which prompted an investigation of the logic. This code does not implement [FIP-0023](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0023.md) correctly. Specifically, if we have the following mintickets:

```
ts1: 1, 2
ts2: 0,3
```

This code will incorrectly return ts1 as the "smaller" tipset, even though the tie should have been broken in favour of ts2 based on `1 > 0`.

* Fix lint error: correct typo in F3Participation function comment

Fix lint error: correct typo in F3Participation function comment

* Fix typo in F3Participate documentation

Corrected "successfull" to "successful" in comments and documentation to fix a lint error.

* chore: libp2p: update to v0.35.4 (#12265)

* go mod tidy

go mod tidy

* Update CHANGELOG.md

Co-authored-by: Steve Loeppky <[email protected]>

* fix a bug

* Set Mainnet upgrade epoch NV23

Set Mainnet upgrade epoch NV23

* Update changelog.md

Update changelog.md

* chore: deps: Update GST, Filecoin-FFI and Actors to final versions NV23 (#12276)

* Update to final GST for nv23

Update to final GST for nv23

* Bump filecoin-ffi to v1.28.0

Bump filecoin-ffi to v1.28.0

* Update CHANGELOG.md

Update CHANGELOG.md

* Update Builtin-actors to v14.0.0

Update Builtin-actors to v14.0.0

* Update Changelog, version-string, make gen and make docsgen

Update Changelog, version-string, make gen and make docsgen

* Re-run ./pack.sh v14 v14.0.0

Re-run ./pack.sh v14 v14.0.0

* Set up otel metrics reporting to prometheus

Register otel collector bridging to the default prometheus registry so
that F3 metrics (which use the newer otel APIs for metrics) get included
in Lotus metrics.

* Update changelog with #12285 backport

Update changelog with #12285 backport

* Revert "chore!: markets: remove deal constants"

This reverts commit d045ccb.

* Revert "chore: config: yet more lp2p removal from miner"

This reverts commit 70400c9.

* fix eth trace panic

* backport: f3: Upgrade to go-f3 `v0.0.5` (#12279) (#12288)

* chore: f3: Upgrade to go-f3 `v0.0.5` (#12279)

* Upgrade to go-f3 `v0.0.5`

Upgrade to the latest version of go-f3 with initial power table storage with far enough lookback, and dynamic manifest persistence.

---------

Co-authored-by: Jakub Sztandera <[email protected]>
Co-authored-by: Jiaying Wang <[email protected]>
Co-authored-by: Steven Allen <[email protected]>

* remove unwanted changelog

* fix import

* Revert "chore!: markets: remove deal constants"

This reverts commit d045ccb.

* Revert "chore: config: yet more lp2p removal from miner"

This reverts commit 70400c9.

* fix eth trace panic

* Update CHANGELOG.md

Co-authored-by: Jakub Sztandera <[email protected]>

* Update CHANGELOG.md

---------

Co-authored-by: Masih H. Derkani <[email protected]>
Co-authored-by: Jakub Sztandera <[email protected]>
Co-authored-by: Steven Allen <[email protected]>
Co-authored-by: Phi <[email protected]>
Co-authored-by: aarshkshah1992 <[email protected]>

---------

Co-authored-by: Jakub Sztandera <[email protected]>
Co-authored-by: Masih H. Derkani <[email protected]>
Co-authored-by: aarshkshah1992 <[email protected]>
Co-authored-by: Rod Vagg <[email protected]>
Co-authored-by: Peter Rabbitson <[email protected]>
Co-authored-by: Aayush Rajasekaran <[email protected]>
Co-authored-by: Steve Loeppky <[email protected]>
Co-authored-by: Jiaying Wang <[email protected]>
Co-authored-by: Steven Allen <[email protected]>
@rjan90 rjan90 mentioned this pull request Aug 31, 2024
8 tasks
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.

4 participants