Skip to content

feat: invariants and tests#595

Merged
0xDiscotech merged 11 commits intosc-feat/fee-splitter-systemfrom
sc-test/fee-splitter-invariants
Sep 26, 2025
Merged

feat: invariants and tests#595
0xDiscotech merged 11 commits intosc-feat/fee-splitter-systemfrom
sc-test/fee-splitter-invariants

Conversation

@simon-something
Copy link
Copy Markdown
Member

Invariants tested:

  • total disbursed fees should always be equal to the sum of the vault balances before disbursement - this is the main invariant, as it eliminate: dust somewhere in the flow, "partial" vault withdrawals (eg one vault not transferring its balance as intended), wrong recipient, etc
  • no dust accumulation in the FeeSplitter
  • disburseFees can only revert if either one of the vault has a balance below it's minimum withdrawal amount or if the disbursement interval has not been reached yet

The share computation isn't tested here, at it would be a copy-paste of the existing fuzzed unit test (perhaps we could increase the fuzz runs for testFuzz_getRecipientsAndAmounts_succeeds, even though it's tightly coupled to the implementation anyway).

To run these efficiently, set corpus_dir = "corpus" in the foundry.toml (it can be run with fail_on_revert too, as revert cases are tested).

cursor[bot]

This comment was marked as outdated.

Copy link
Copy Markdown

@0xDiscotech 0xDiscotech left a comment

Choose a reason for hiding this comment

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

Looking good so far! I'd like to ask coverage on the following branches/cases:

  • Could we add a setter updating the minWithdrawalAmount threshold? We are way more interested on testing the 0 amount scenario than anything else, but we still need to cover other branches or paths.

  • Could we add also tests checking that L1Withdrawer withdraws when the threshold is reached, and that FeesDepositor initiates the deposit when the threshold is reached as well? No strictly needed if you don't find any invariant matching this test, but I think it'd be good to cover that integration as well.

@simon-something
Copy link
Copy Markdown
Member Author

Looking good so far! I'd like to ask coverage on the following branches/cases:

  • Could we add a setter updating the minWithdrawalAmount threshold? We are way more interested on testing the 0 amount scenario than anything else, but we still need to cover other branches or paths.

Choure fing!

  • Could we add also tests checking that L1Withdrawer withdraws when the threshold is reached, and that FeesDepositor initiates the deposit when the threshold is reached as well? No strictly needed if you don't find any invariant matching this test, but I think it'd be good to cover that integration as well.

For the L1Withdrawer, it's already used (hence the L2toL1 balance included in the token conservation), but will add more granular test to make sure it get triggered on threshold (I saw it in the trace, but that doesn't count;)
Will add FeesDepositor too, shouldn't be a big lift

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@simon-something simon-something force-pushed the sc-test/fee-splitter-invariants branch from c0ffee5 to c0ffee4 Compare September 26, 2025 09:32
cursor[bot]

This comment was marked as outdated.

Copy link
Copy Markdown

@0xDiscotech 0xDiscotech left a comment

Choose a reason for hiding this comment

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

LGTM!

@0xDiscotech 0xDiscotech merged commit 5a0d229 into sc-feat/fee-splitter-system Sep 26, 2025
2 checks passed
@0xDiscotech 0xDiscotech deleted the sc-test/fee-splitter-invariants branch September 26, 2025 18:07
agusduha pushed a commit that referenced this pull request Nov 7, 2025
* feat: fee splitter system (#469)


----

Co-authored-by: Disco <131301107+0xDiscotech@users.noreply.github.com>
Co-authored-by: Flux <175354924+0xiamflux@users.noreply.github.com>
Co-authored-by: Chiin <77933451+0xChin@users.noreply.github.com>

* fix: failing tests (#553)

* refactor: revenue sharing config (#538)

* fix: pre pr and semgrep (#565)

* fix: warnings (#571)

* fix: tests failing on fork environment (#575)

* fix: use encode call on constructors (#578)

* feat: add integration splitter test (#581)

* fix: integration test nits (#583)

* test: more coverage on splitter tests (#594)


---------

Co-authored-by: Chiin <77933451+0xChin@users.noreply.github.com>

* fix: add missing operator fee vault field (#616)

* feat: invariants and tests (#595)

* fix: ir informationals (#609)

* fix: pre pr (#622)

* refactor: use cdm (#624)

* fix: update withdrawal gas limit value on check (#627)

* fix: remove rev share field from deploy op chain input struct (#628)

* refactor: remove initializer and vaults getter changes (#631)

* fix: remove immutable check over vaults (#634)


Co-authored-by: 0xng <87835144+0xng@users.noreply.github.com>
Co-authored-by: Funkornaut <107587461+funkornaut001@users.noreply.github.com>
Co-authored-by: Joxess <91908708+Joxess@users.noreply.github.com>

* fix: audit findings (#658)

* fix: pre pr

* feat: add user guide docs for rev sharing on op deployer (#666)

* docs: add create2 comment (#667)

---------

Co-authored-by: Funkornaut <107587461+funkornaut001@users.noreply.github.com>
Co-authored-by: Flux <175354924+0xiamflux@users.noreply.github.com>
Co-authored-by: Chiin <77933451+0xChin@users.noreply.github.com>
Co-authored-by: Simon Something /DrGoNoGo <83670532+simon-something@users.noreply.github.com>
Co-authored-by: 0xng <87835144+0xng@users.noreply.github.com>
Co-authored-by: Joxess <91908708+Joxess@users.noreply.github.com>
0xDiscotech added a commit that referenced this pull request Dec 2, 2025
* feat: fee splitter system (#469)

----

Co-authored-by: Disco <131301107+0xDiscotech@users.noreply.github.com>
Co-authored-by: Flux <175354924+0xiamflux@users.noreply.github.com>
Co-authored-by: Chiin <77933451+0xChin@users.noreply.github.com>

* fix: failing tests (#553)

* refactor: revenue sharing config (#538)

* fix: pre pr and semgrep (#565)

* fix: warnings (#571)

* fix: tests failing on fork environment (#575)

* fix: use encode call on constructors (#578)

* feat: add integration splitter test (#581)

* fix: integration test nits (#583)

* test: more coverage on splitter tests (#594)

---------

Co-authored-by: Chiin <77933451+0xChin@users.noreply.github.com>

* fix: add missing operator fee vault field (#616)

* feat: invariants and tests (#595)

* fix: ir informationals (#609)

* fix: pre pr (#622)

* refactor: use cdm (#624)

* fix: update withdrawal gas limit value on check (#627)

* fix: remove rev share field from deploy op chain input struct (#628)

* refactor: remove initializer and vaults getter changes (#631)

* fix: remove immutable check over vaults (#634)

Co-authored-by: 0xng <87835144+0xng@users.noreply.github.com>
Co-authored-by: Funkornaut <107587461+funkornaut001@users.noreply.github.com>
Co-authored-by: Joxess <91908708+Joxess@users.noreply.github.com>

* fix: audit findings (#658)

* fix: pre pr

* feat: add user guide docs for rev sharing on op deployer (#666)

* docs: add create2 comment (#667)

---------

Co-authored-by: Funkornaut <107587461+funkornaut001@users.noreply.github.com>
Co-authored-by: Flux <175354924+0xiamflux@users.noreply.github.com>
Co-authored-by: Chiin <77933451+0xChin@users.noreply.github.com>
Co-authored-by: Simon Something /DrGoNoGo <83670532+simon-something@users.noreply.github.com>
Co-authored-by: 0xng <87835144+0xng@users.noreply.github.com>
Co-authored-by: Joxess <91908708+Joxess@users.noreply.github.com>
0xDiscotech added a commit that referenced this pull request Dec 2, 2025
* feat: fee splitter system (#469)

----

Co-authored-by: Disco <131301107+0xDiscotech@users.noreply.github.com>
Co-authored-by: Flux <175354924+0xiamflux@users.noreply.github.com>
Co-authored-by: Chiin <77933451+0xChin@users.noreply.github.com>

* fix: failing tests (#553)

* refactor: revenue sharing config (#538)

* fix: pre pr and semgrep (#565)

* fix: warnings (#571)

* fix: tests failing on fork environment (#575)

* fix: use encode call on constructors (#578)

* feat: add integration splitter test (#581)

* fix: integration test nits (#583)

* test: more coverage on splitter tests (#594)

---------

Co-authored-by: Chiin <77933451+0xChin@users.noreply.github.com>

* fix: add missing operator fee vault field (#616)

* feat: invariants and tests (#595)

* fix: ir informationals (#609)

* fix: pre pr (#622)

* refactor: use cdm (#624)

* fix: update withdrawal gas limit value on check (#627)

* fix: remove rev share field from deploy op chain input struct (#628)

* refactor: remove initializer and vaults getter changes (#631)

* fix: remove immutable check over vaults (#634)

Co-authored-by: 0xng <87835144+0xng@users.noreply.github.com>
Co-authored-by: Funkornaut <107587461+funkornaut001@users.noreply.github.com>
Co-authored-by: Joxess <91908708+Joxess@users.noreply.github.com>

* fix: audit findings (#658)

* fix: pre pr

* feat: add user guide docs for rev sharing on op deployer (#666)

* docs: add create2 comment (#667)

---------

Co-authored-by: Funkornaut <107587461+funkornaut001@users.noreply.github.com>
Co-authored-by: Flux <175354924+0xiamflux@users.noreply.github.com>
Co-authored-by: Chiin <77933451+0xChin@users.noreply.github.com>
Co-authored-by: Simon Something /DrGoNoGo <83670532+simon-something@users.noreply.github.com>
Co-authored-by: 0xng <87835144+0xng@users.noreply.github.com>
Co-authored-by: Joxess <91908708+Joxess@users.noreply.github.com>
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