Skip to content

chore(fees): add initial fees & migration testing (FIP-0100)#12942

Merged
rvagg merged 2 commits intorvagg/calibnet-cs-adjustfrom
rvagg/fip-0100-migration
Mar 14, 2025
Merged

chore(fees): add initial fees & migration testing (FIP-0100)#12942
rvagg merged 2 commits intorvagg/calibnet-cs-adjustfrom
rvagg/fip-0100-migration

Conversation

@rvagg
Copy link
Copy Markdown
Member

@rvagg rvagg commented Mar 10, 2025

FIP-0100 fees migration test. Still some more to add here and I'm stacking up PRs a little. This one is on top of #12938 which sets up circulating supply.

This PR uses UnmanagedMiner and has some changes I'd like to get merged regardless of FIP-0100's status. Currently we're testing onboarding different kinds of sectors while in nv24, then hitting the upgrade and snapping into CC sectors and checking fees. Also need to add new sectors post-nv25 and probably an extension or two. I also still haven't figured out how to test actual fee deduction in a reliable way, and it would also be good to do that with the BR cap too if I can figure out how to manage that as well.

@github-project-automation github-project-automation Bot moved this to 📌 Triage in FilOz Mar 10, 2025
@rvagg rvagg changed the title chore(fees): add initial fees migration test chore(fees): add initial fees migration test (FIP-0100) Mar 10, 2025
@rvagg rvagg force-pushed the rvagg/calibnet-cs-adjust branch 2 times, most recently from 69ad783 to 28e6193 Compare March 10, 2025 09:13
@rvagg rvagg force-pushed the rvagg/fip-0100-migration branch 2 times, most recently from 09aa600 to 5fb2d67 Compare March 10, 2025 09:16
@rvagg rvagg changed the title chore(fees): add initial fees migration test (FIP-0100) chore(fees): add initial fees & migration testing (FIP-0100) Mar 10, 2025
@rvagg rvagg force-pushed the rvagg/fip-0100-migration branch from 5fb2d67 to 71f9107 Compare March 10, 2025 09:51
@rvagg rvagg force-pushed the rvagg/calibnet-cs-adjust branch from 28e6193 to 33c6ba5 Compare March 11, 2025 01:15
@rvagg rvagg force-pushed the rvagg/fip-0100-migration branch from 71f9107 to 7d4c10f Compare March 11, 2025 01:16
@rvagg rvagg force-pushed the rvagg/calibnet-cs-adjust branch from 33c6ba5 to 17ef76e Compare March 11, 2025 01:52
@rvagg rvagg force-pushed the rvagg/fip-0100-migration branch from 7d4c10f to 0901fd1 Compare March 11, 2025 02:42
@rvagg rvagg force-pushed the rvagg/calibnet-cs-adjust branch from 17ef76e to 9d65e33 Compare March 11, 2025 03:53
@rvagg rvagg force-pushed the rvagg/fip-0100-migration branch 2 times, most recently from 6380873 to 77baaa1 Compare March 11, 2025 06:18
@rvagg rvagg marked this pull request as ready for review March 11, 2025 06:50
@rvagg rvagg added the skip/changelog This change does not require CHANGELOG.md update label Mar 11, 2025
@rvagg
Copy link
Copy Markdown
Member Author

rvagg commented Mar 11, 2025 •
edited
Loading

Calling this ready for review. Still depends on the calibnet (etc.) CS fix @ #12938 but this is exercising the fees.

  1. Onboard 2xCC, deal & verified sectors within nv24, assert no fees and a 15 field SectorOnChainInfo
  2. Upgrade to nv25, observe no changes
  3. Snap the CC sectors, one with an unverified and one with a verified piece; assert that they get the proper fees assigned to them - also assert that all sectors now have the 16 field SectorOnChainInfo—they are all in the root of the Sectors HAMT so get overwritten, but the others have 0 fees
  4. Onboard another set of 2xCC, deal & verified sectors, assert they all have the correct fees
  5. Wait for each of the sectors with fees to have passed a deadline and check that the miner balance is deducted by the exact expected amount (total of all fees)

Limitations:

  1. This is with UnmanagedMiner which doesn't (yet) do WinningPoST, so never has any vesting funds, so we only observe it being taken from balance.
  2. No tests for nv24 sectors being extended in nv25 and attracting a fee
  3. No change-QAP tests other than the nv24 snaps in nv25 (I could add nv25 snapping to change QAP, but accounting starts to get complicated as I cross more proving windows)
  4. No inspection of deadline state
  5. No handling of expirations and looking at fee_deduction

@rvagg rvagg requested review from Stebalien and ZenGround0 March 11, 2025 06:56
This was linked to issues Mar 11, 2025
Integration test for FIP-0100 sector onboarding fee #12899
Closed
Expose "APIs" for accessing FIP-0100 onboarding fee info #12900
Closed
@rvagg rvagg force-pushed the rvagg/fip-0100-migration branch 3 times, most recently from 7333de9 to 4d4b1e5 Compare March 11, 2025 09:33
@BigLep
Copy link
Copy Markdown
Member

BigLep commented Mar 11, 2025

Awesome work @rvagg !

Limitations

Which (if any) of these do you think we should get in before calling this done?

I'm assuming "nv24 sectors being extended in nv25 and attracting a fee" would be the most valuable addition given that is a top concern area for existing SPs.

@rvagg rvagg force-pushed the rvagg/fip-0100-migration branch from 3ebed51 to 2315284 Compare March 12, 2025 05:29
@BigLep BigLep moved this from 📌 Triage to 🔎 Awaiting Review in FilOz Mar 13, 2025
@BigLep
Copy link
Copy Markdown
Member

BigLep commented Mar 13, 2025

@Stebalien or @ZenGround0 : will you be able to look at this today (2025-03-13) so @rvagg can incorporate feedback and get this merged by end of the week?

@Stebalien Stebalien mentioned this pull request Mar 13, 2025
Merged
8 tasks
@Stebalien
Copy link
Copy Markdown
Member

Stebalien commented Mar 13, 2025

Test fix in #12950.

Stebalien
Stebalien reviewed Mar 14, 2025
View reviewed changes
Copy link
Copy Markdown
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.

Some nits but this seems reasonable. It would also be nice if we could run the invariants checks in the test, just to make sure they're sane.

Comment thread chain/actors/builtin/miner/miner.go Outdated
Comment thread itests/niporep_manual_test.go Outdated
Comment thread itests/manual_onboarding_test.go Outdated
Comment thread itests/manual_onboarding_test.go Outdated
Comment thread itests/kit/node_unmanaged.go Outdated
@github-project-automation github-project-automation Bot moved this from 🔎 Awaiting Review to ✔️ Approved by reviewer in FilOz Mar 14, 2025
@rvagg
Copy link
Copy Markdown
Member Author

rvagg commented Mar 14, 2025

(all those review items addressed in latest commit btw)

@rvagg rvagg merged commit 32b0a3e into rvagg/calibnet-cs-adjust Mar 14, 2025
@rvagg rvagg deleted the rvagg/fip-0100-migration branch March 14, 2025 04:06
@github-project-automation github-project-automation Bot moved this from ✔️ Approved by reviewer to 🎉 Done in FilOz Mar 14, 2025
@github-project-automation github-project-automation Bot moved this from 🔎 Awaiting Review to 🎉 Done in nv25 Track Board Mar 14, 2025
rvagg added a commit that referenced this pull request Mar 14, 2025
@rvagg
chore(fees): add initial fees & migration testing (FIP-0100) (#12942)
f397a87
rvagg added a commit that referenced this pull request Mar 14, 2025
@rvagg
chore(fees): add initial fees & migration testing (FIP-0100) (#12942)
98f4e68
rvagg added a commit that referenced this pull request Mar 17, 2025
@rvagg
chore(fees): add initial fees & migration testing (FIP-0100) (#12942)
b14d3a7
This was referenced Mar 18, 2025
Closed
Closed
rvagg added a commit that referenced this pull request Mar 18, 2025
@rvagg
chore(fees): add initial fees & migration testing (FIP-0100) (#12942)
9d70a8c
rvagg added a commit that referenced this pull request Mar 18, 2025
@rvagg
chore(fees): add initial fees & migration testing (FIP-0100) (#12942)
3edc2b8
rvagg added a commit that referenced this pull request Mar 18, 2025
@rvagg
chore(fees): add initial fees & migration testing (FIP-0100) (#12942)
5d71d74
rvagg added a commit that referenced this pull request Mar 18, 2025
@rvagg
chore(fees): add initial fees & migration testing (FIP-0100) (#12942)
10f8c5b
rvagg added a commit that referenced this pull request Mar 18, 2025
@rvagg
chore(fees): add initial fees & migration testing (FIP-0100) (#12942)
8469c05
@rjan90 rjan90 moved this from 🎉 Done to ☑️ Done (Archive) in FilOz Mar 18, 2025
rvagg added a commit that referenced this pull request Mar 19, 2025
@rvagg
chore(fees): add initial fees & migration testing (FIP-0100) (#12942)
7631d6f
rvagg added a commit that referenced this pull request Mar 19, 2025
@rvagg
chore(fees): add initial fees & migration testing (FIP-0100) (#12942)
637e215
@rjan90 rjan90 mentioned this pull request Mar 19, 2025
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Stebalien Stebalien Stebalien approved these changes

@ZenGround0 ZenGround0 Awaiting requested review from ZenGround0

Assignees

@rvagg rvagg

Labels

skip/changelog This change does not require CHANGELOG.md update

Projects

FilOz
Status: ☑️ Done (Archive)
nv25 Track Board
Status: 🎉 Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Expose "APIs" for accessing FIP-0100 onboarding fee info Integration test for FIP-0100 sector onboarding fee

4 participants

@rvagg @BigLep @Stebalien @rjan90