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

fix: miner: Fix DDO pledge math #12341

Merged
merged 5 commits into from
Aug 13, 2024
Merged

fix: miner: Fix DDO pledge math #12341

merged 5 commits into from
Aug 13, 2024

Conversation

magik6k
Copy link
Contributor

@magik6k magik6k commented Aug 5, 2024

Related Issues

Proposed Changes

  • Add a bunch of logging around pledge calculation to aid with further debugging if need be
  • Change pledge calculation in handleSubmitCommitAggregate to use DDO-aware function

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, 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

@rjan90
Copy link
Contributor

rjan90 commented Aug 6, 2024

This PR needs a entry to the changelog to pass the Changelog-check.

@magik6k magik6k mentioned this pull request Aug 9, 2024
Open
@magik6k
Copy link
Contributor Author

magik6k commented Aug 9, 2024

This PR now also pulls in #12342.

I have tested the equivalent changes in Curio on mainnet where those fixes seem to solve all the issues.

I don't think this is possible to meaningfully test in itests because:

  • itest econ shifts too fast to catch pledge mismatches
  • itest miner vests rewards way faster than any impacts on balance from pledge.

So ideally someone would give this a try on a real setup and report what messages were sent to the chain.

@LexLuthr LexLuthr mentioned this pull request Aug 13, 2024
Closed
8 tasks
@rjan90 rjan90 mentioned this pull request Aug 13, 2024
Closed
23 tasks
@rjan90 rjan90 merged commit 415e7e6 into master Aug 13, 2024
85 checks passed
@rjan90 rjan90 deleted the fix/ddo-pledge branch August 13, 2024 12:34
@rvagg rvagg mentioned this pull request Aug 14, 2024
Merged
@rjan90 rjan90 mentioned this pull request Aug 14, 2024
Closed
8 tasks
rvagg pushed a commit that referenced this pull request Aug 14, 2024
@magik6k @ZenGround0 @rvagg
fix: miner: Fix DDO pledge math (#12341)
be0c4cc 
* Power is units of Space * Time so multiply by deal duration

* fix: miner: Fix DDO pledge math

* appease the changelog checker

* Fix gen

---------

Co-authored-by: zenground0 <[email protected]>
@rvagg rvagg mentioned this pull request Aug 14, 2024
Closed
rjan90 pushed a commit that referenced this pull request Aug 15, 2024
@magik6k @ZenGround0 @rjan90
fix: miner: Fix DDO pledge math (#12341)
61667b4 
* Power is units of Space * Time so multiply by deal duration

* fix: miner: Fix DDO pledge math

* appease the changelog checker

* Fix gen

---------

Co-authored-by: zenground0 <[email protected]>
rjan90 pushed a commit that referenced this pull request Aug 15, 2024
@magik6k @ZenGround0 @rjan90
fix: miner: Fix DDO pledge math (#12341)
0cc01e0 
* Power is units of Space * Time so multiply by deal duration

* fix: miner: Fix DDO pledge math

* appease the changelog checker

* Fix gen

---------

Co-authored-by: zenground0 <[email protected]>
rjan90 pushed a commit that referenced this pull request Aug 15, 2024
@magik6k @ZenGround0 @rjan90
fix: miner: Fix DDO pledge math (#12341)
191d88e 
* Power is units of Space * Time so multiply by deal duration

* fix: miner: Fix DDO pledge math

* appease the changelog checker

* Fix gen

---------

Co-authored-by: zenground0 <[email protected]>
@rjan90 rjan90 mentioned this pull request Aug 15, 2024
Closed
8 tasks
rjan90 pushed a commit that referenced this pull request Aug 15, 2024
@magik6k @ZenGround0 @rjan90
fix: miner: Fix DDO pledge math (#12341)
e13ea92 
* Power is units of Space * Time so multiply by deal duration

* fix: miner: Fix DDO pledge math

* appease the changelog checker

* Fix gen

---------

Co-authored-by: zenground0 <[email protected]>
@rjan90 rjan90 mentioned this pull request Aug 15, 2024
Merged
8 tasks
rjan90 pushed a commit that referenced this pull request Aug 15, 2024
@magik6k @ZenGround0 @rjan90
fix: miner: Fix DDO pledge math (#12341)
69225e3 
* Power is units of Space * Time so multiply by deal duration

* fix: miner: Fix DDO pledge math

* appease the changelog checker

* Fix gen

---------

Co-authored-by: zenground0 <[email protected]>
jennijuju pushed a commit that referenced this pull request Aug 15, 2024
@magik6k @ZenGround0 @jennijuju
fix: miner: Fix DDO pledge math (#12341)
6cc6de3 
* Power is units of Space * Time so multiply by deal duration

* fix: miner: Fix DDO pledge math

* appease the changelog checker

* Fix gen

---------

Co-authored-by: zenground0 <[email protected]>
rjan90 added a commit that referenced this pull request Aug 19, 2024
@rjan90 @LexLuthr
@Kubuxu @jennijuju @Stebalien @rvagg @aarshkshah1992 @magik6k @ZenGround0 @BigLep
chore: post release steps for #12379 (v1.28.2 miner and node patch re…
0225c91 
…lease) (#12400)

* fix: lotus-miner: remove provecommit1 method (#12251)

* remove provecommit1

* add changelog

* update precommit and commit params

* fix lint error

* fix commit params

* dep: f3: Update go-f3 to 0.0.6, enable it on mainnet (#12295)

* Update go-f3 to 0.0.6

Signed-off-by: Jakub Sztandera <[email protected]>

* Enable F3 in passive configuration in mainnet config

Signed-off-by: Jakub Sztandera <[email protected]>

* Add changelog

Signed-off-by: Jakub Sztandera <[email protected]>

* add new butterfly assets

---------

Signed-off-by: Jakub Sztandera <[email protected]>
Co-authored-by: Jennifer Wang <[email protected]>

* retract v1.28.0

* update v1.28.0 changelog and add v1.28.1

* Update CHANGELOG.md

* wip - update f3

* don't convert bigint type

We now use the same one in GPBFT.

* update docs

* fix wrong param name

* update butterfy assets

* update go-f3

* update changelog

* update version

* fix typo

* Update CHANGELOG.md

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

* Update CHANGELOG.md

Co-authored-by: Rod Vagg <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Rod Vagg <[email protected]>

* apply f3 patch

* chore: bump versions and make gen/docsgen-cli

chore: bump versions and make gen/docsgen-cli

* chore: update v1.28.2 changelog

chore: update v1.282. changelog

* feat: f3: update go-f3 to 0.2.0 (#12390)

* Update go-f3 to 0.2.0

Includes:
 - fix for excessive bandwidth usage
 - significant performance improvements
 - minor consensus fixes

Signed-off-by: Jakub Sztandera <[email protected]>

* add changelog

Signed-off-by: Jakub Sztandera <[email protected]>

* chore(f3): update to final released version

---------

Signed-off-by: Jakub Sztandera <[email protected]>
Co-authored-by: Steven Allen <[email protected]>

* fix!: sealer: handle initialisation error without panic

storage/pipeline.NewPreCommitBatcher and storage/pipeline.New now have an additional
error return to deal with errors arising from fetching the sealing config.

* add breaking API upgrade warning to the ChangeLog

* NewCommitBatcher now has an additional
error return to deal with errors arising from fetching the sealing config.

* fix: miner: Fix DDO pledge math (#12341)

* Power is units of Space * Time so multiply by deal duration

* fix: miner: Fix DDO pledge math

* appease the changelog checker

* Fix gen

---------

Co-authored-by: zenground0 <[email protected]>

* chore: fix lint error

- Updated the logging statement in `testOutOfGasError` to correctly reference `build.BlockGasLimit` instead of `buildconstants.BlockGasLimit`.

* fix: update changelog to reference bandwidth issue ticket

fix: update changelog to reference bandwidth issue ticket

* Update CHANGELOG.md

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

* Update CHANGELOG.md

* chore: make gen and make docsgen-cli

Run `make gen` and `make docsgen-cli`

---------

Signed-off-by: Jakub Sztandera <[email protected]>
Co-authored-by: LexLuthr <[email protected]>
Co-authored-by: Jakub Sztandera <[email protected]>
Co-authored-by: Jennifer Wang <[email protected]>
Co-authored-by: Jiaying Wang <[email protected]>
Co-authored-by: Steven Allen <[email protected]>
Co-authored-by: Rod Vagg <[email protected]>
Co-authored-by: aarshkshah1992 <[email protected]>
Co-authored-by: Łukasz Magiera <[email protected]>
Co-authored-by: zenground0 <[email protected]>
Co-authored-by: Steve Loeppky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rvagg rvagg rvagg 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.
4 participants
@magik6k @rjan90 @rvagg @ZenGround0