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

fix: params: Update SupportedProofTypes to the correct ones #11988

Merged
merged 1 commit into from
May 13, 2024

Conversation

rjan90
Copy link
Contributor

@rjan90 rjan90 commented May 12, 2024

It was noted by Hubert that Filecoin.StateGetNetworkParams returns the wrong SupportedProofTypes:

Example:

curl -X POST 'http://127.0.0.1:1234/rpc/v0' \
>     -H 'Content-Type: application/json' \
>     --data '{"jsonrpc":"2.0","id":1,"method":"Filecoin.StateGetNetworkParams","params":[]}'
{"jsonrpc":"2.0","result":{"NetworkName":"mainnet","BlockDelaySecs":30,"ConsensusMinerMinPower":"10995116277760","SupportedProofTypes":[3,4],"PreCommitChallengeDelay":150,"
------
UpgradeWatermelonHeight":3469380,"UpgradeDragonHeight":3855360,"UpgradePhoenixHeight":3855480},"Eip155ChainID":314},"id":1}

Proposed Changes

Update the SupportedProofsTypes in build/params_* to the correct set of ProofTypes supported in the different networks.

Additional Info

Given that nobody has not complained about this earlier, it brings the question if this field is worth keeping?

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

Update SupportedProofTypes to the correct ones.
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

I assume these changed at some network upgrade, but the method doesn't take an epoch, tipset or anything else that could be used to give different results. So this change seems reasonable given "now" is the only reasonable way of interpreting a request.

@rjan90 rjan90 merged commit 7e005be into master May 13, 2024
184 of 186 checks passed
@rjan90 rjan90 deleted the fix/Update-SupportedProofsTypes branch May 13, 2024 07:43
rjan90 added a commit that referenced this pull request May 13, 2024
rjan90 added a commit that referenced this pull request May 14, 2024
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.

2 participants