Remove Batch Fee For Precommit and Lower BatchBalancer #1094

irenegia · 2024-12-19T16:39:18Z

We are proposing a FIP to support more batching/aggregation as low hanging fruit to give some onboarding gas saving in the short term

cc @AxCortesCubero @zixuanzh

@AxCortesCubero

We are proposing a FIP to support more batching/aggregation as low hanging fruit to give some onboarding gas saving in the short term cc @AxCortesCubero @zixuanzh

lanzafame

Lacking some reasoning for the chosen numbers for the batchBalancer but otherwise seems good.

FIPS/fip-lowerbatchBal.md

lanzafame · 2024-12-19T23:14:37Z

FIPS/fip-lowerbatchBal.md

+At precommit there are two options: (1) "simple" batching as in precommit (ie, one message for ≥ 2 sectors) where there is a porep proof for each sector in the batch, and (2) batching with aggregation, where there is one unique aggregated porep proof.
+Batching for provecommit is already rational (cost-effective), as the batch fee applies only to aggregated proofs (opposed to precommit, where the batch balancer applies to batching). 
+
+On the other hand, aggregation is rational only when the base fee exceeds 0.065 nanoFIL. Lowering the value of `batchBalancer` from 5 to 2 nanoFIL will reduce the crossover point to 0.026 nanoFIL, making the aggregation rational across a broader range of base fee values.


Why 2 nanoFIL? And why 0.026 nanoFIL for the crossover? Any modelling to back these choices versus other values?

More info here https://docs.google.com/document/d/17FYnKIGf1uf1cCUwfKd30pQpTq8fAux5YghRIlfqrdE/edit?tab=t.0#bookmark=id.4gq3p5g7oyn8

could you @ZX @AxCortesCubero please add a short recap of that to the FIP text?

FIPS/fip-lowerbatchBal.md

rvagg · 2024-12-20T02:08:51Z

@irenegia very tiny nit: it might be good to make the casing of "batchBalancer" more consistent across the doc. In the authoritative code (builtin-actors), it's BATCH_BALANCER, in go-state-types it's BatchBalancer. I don't think there's a good reason to camelCase it, perhaps "BatchBalancer" would be best. You do have aggregate_fee in there, which is from builtin-actors and matches.

FIPS/fip-lowerbatchBal.md

jennijuju · 2024-12-21T04:42:46Z

FIPS/fip-lowerbatchBal.md

+fip: "00XX"
+title: Remove Batch Fee For Precommit and Lower batchBalancer
+author: irene, ZX, AX
+discussions-to: https://github.com/filecoin-project/FIPs/discussions/1092


Could you please create a separate discussion dedicated to this proposal - so that it is easier to capture a discussion that is specific to what is proposed here?

FIPS/fip-lowerbatchBal.md

jennijuju · 2024-12-21T05:21:35Z

FIPS/fip-lowerbatchBal.md

+At precommit there are two options: (1) "simple" batching as in precommit (ie, one message for ≥ 2 sectors) where there is a porep proof for each sector in the batch, and (2) batching with aggregation, where there is one unique aggregated porep proof.
+Batching for provecommit is already rational (cost-effective), as the batch fee applies only to aggregated proofs (opposed to precommit, where the batch balancer applies to batching). 
+
+On the other hand, aggregation is rational only when the base fee exceeds 0.065 nanoFIL. Lowering the value of `batchBalancer` from 5 to 2 nanoFIL will reduce the crossover point to 0.026 nanoFIL, making the aggregation rational across a broader range of base fee values.


Suggested change

On the other hand, aggregation is rational only when the base fee exceeds 0.065 nanoFIL. Lowering the value of `batchBalancer` from 5 to 2 nanoFIL will reduce the crossover point to 0.026 nanoFIL, making the aggregation rational across a broader range of base fee values.

On the other hand, aggregation is rational only when the base fee exceeds 0.065 nanoFIL today. Lowering the value of `batchBalancer` from 5 to 2 nanoFIL will reduce the crossover point to 0.026 nanoFIL, making the aggregation rational across a broader range of base fee values.

Can you specify how you get the 0.065 nanoFIL number? If this is true - it sounds like SP client like lotus-miner should change their default immediately or miners should manually update their cocnfig so that aggregation is enabled whenever base fee is higher than 0.065 nanoFIL and start saving (and the network does have >0.065nanoFIL base fee sometimes atm).

The value 0.065 nanoFIL was computed by @zixuanzh using the new cost (gas units) of PCS3, more details can be found here

jennijuju · 2024-12-21T05:32:26Z

FIPS/fip-lowerbatchBal.md

+- [Msg 2](https://www.filutils.com/en/message/bafy2bzaceah7m6jzravjoswo2pljzit36euu3sgz5jzbnpkcfp23b76texiv6): ProveCommitSectors3 for 4 sectors, batched (no aggregation): 96.M/4 = 24M per sector 
+- [Msg 3](https://www.filutils.com/en/message/bafy2bzacedeh74ds4x4l5nlfahmlvwn4obfukhgqnf6rxlaargvsm56sljune): ProveCommitSectors3 for 4 sectors, aggregated[^*]: 217M/4 = 54M per sector 
+
+[^*]: In practice, aggregation is currently giving less gas saving respect to batching due to a bug causing single proofs to be charged an incorrect amount of gas units. So currently, batching is to be preferred with respect to aggregation. Once this code bug is fixed, batching will cost more gas units and aggregation would be the option with the largest gas unit saving per sector. 


Suggested change

[^*]: In practice, aggregation is currently giving less gas saving respect to batching due to a bug causing single proofs to be charged an incorrect amount of gas units. So currently, batching is to be preferred with respect to aggregation. Once this code bug is fixed, batching will cost more gas units and aggregation would be the option with the largest gas unit saving per sector.

[^*]: In practice, aggregation is currently giving less gas saving respect to batching due to a bug causing single proofs to be charged an incorrect amount of gas units. So currently, batching is to be preferred with respect to aggregation. Once this bug in the ref-fvm implementation is addressed, proof aggregation will be the most gas-efficient approach if one can onboard 4 or more sectors at the same time.

can onboard 4 or more sectors at the same time.

technical: @irenegia can you confirm whether this is still correct?

ack, working on that

jennijuju · 2024-12-21T05:41:25Z

On a high level, this FIP is technically sound. However, I think it needs to document how the authors are getting/choosing the numbers mentioned in this FIP better & some editorial fixes.

Co-authored-by: Adrian Lanzafame <[email protected]> Co-authored-by: Jiaying Wang <[email protected]>

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

Co-authored-by: Jiaying Wang <[email protected]> Co-authored-by: Adrian Lanzafame <[email protected]>

irenegia · 2024-12-23T14:07:36Z

@irenegia very tiny nit: it might be good to make the casing of "batchBalancer" more consistent across the doc. In the authoritative code (builtin-actors), it's BATCH_BALANCER, in go-state-types it's BatchBalancer. I don't think there's a good reason to camelCase it, perhaps "BatchBalancer" would be best. You do have aggregate_fee in there, which is from builtin-actors and matches.

Changed to BatchBalancer everywhere

editorial changes + added links to precommit and provecommit msgs with deals

Create fip-lowerbatchBal.md

54574ff

We are proposing a FIP to support more batching/aggregation as low hanging fruit to give some onboarding gas saving in the short term cc @AxCortesCubero @zixuanzh

irenegia requested review from momack2, arajasek, jennijuju, kaitlin-beegle, anorth, raulk, jsoares, TippyFlitsUK and rvagg as code owners December 19, 2024 16:39

Update README.md

627aaa6

lanzafame reviewed Dec 19, 2024

View reviewed changes

rvagg reviewed Dec 20, 2024

View reviewed changes