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

commit batch: AggregateAboveBaseFee config #6650

Merged
merged 2 commits into from
Jul 1, 2021
Merged

Conversation

magik6k
Copy link
Contributor

@magik6k magik6k commented Jul 1, 2021

Closes #6645

This PR adds AggregateAboveBaseFee to the Sealing config. The value is used to decide whether or not to do aggregation based on the current network basefee. Defaults to 0.15nFIL based on estimates from https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0013.md#batch-incentive-alignment

(also adds a regression test to #6647)

@magik6k magik6k mentioned this pull request Jul 1, 2021
10 tasks
@magik6k magik6k requested a review from arajasek July 1, 2021 13:50
@@ -144,6 +144,10 @@ type SealingConfig struct {
// time buffer for forceful batch submission before sectors/deals in batch would start expiring
CommitBatchSlack Duration

// network BaseFee below which to stop doing commit aggregation, instead
// submitting proofs to the chain individually
AggregateAboveBaseFee types.FIL
Copy link
Member

Choose a reason for hiding this comment

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

add the default unit please? attofil?nanofil?.. including a table in the pr description on how to set the value in each unit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's basefee in FIL (this is what's types.FIL is)

@jennijuju
Copy link
Member

lets update https://docs.filecoin.io/mine/lotus/miner-configuration/#sealing-section please?

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.

LGTM, ship

flush(getSectors(2), true, false),
},
},
"addAte-aboveBalancer": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ate?

},
},
"addAte": {
"addAte-belowBalancer": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ate.

@magik6k magik6k merged commit 20c8250 into master Jul 1, 2021
@magik6k magik6k deleted the feat/agg-balancer-config branch July 1, 2021 16:36
@stuberman
Copy link

I updated to latest Master - is there a setting in config.toml to adjust the network basefee parameter from 0.15 nFIL?

@magik6k
Copy link
Contributor Author

magik6k commented Jul 2, 2021

Yes, this adds AggregateAboveBaseFee to the Sealing config (value in FIL, not nFIL)

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.

Trigger AggregateCommit dynamically base on base fee
5 participants