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

Feat simplify onAfterAllocateLQTY hook #12

Closed
wants to merge 4 commits into from

Conversation

GalloDaSballo
Copy link
Collaborator

@GalloDaSballo GalloDaSballo commented Oct 7, 2024

  • Add assert for onAfterAllocateLQTY to ensure the code works as the spec

  • Rewrite the code to be easier to read

  • Simplify the code

  • Implement tests for it

  • E2E / Fuzz for reverts

  • Invariant Tests for accounting and reverts

  • Fix Tests
    These tests are currently broken:

Ran 8 test suites in 2.22s (2.21s CPU time): 37 tests passed, 6 failed, 0 skipped (43 total tests)

Failing tests:
Encountered 6 failing tests in test/BribeInitiativeAllocate.t.sol:BribeInitiativeAllocateTest
[FAIL. Reason: assertion failed: 2001000000000000000000 != 1000000000000000000] test_onAfterAllocateLQTY_newEpoch_NoVetoToVeto() (gas: 403990)
[FAIL. Reason: assertion failed: 2001000000000000000000 != 1000000000000000000] test_onAfterAllocateLQTY_newEpoch_VetoToNoVeto() (gas: 252067)
[FAIL. Reason: assertion failed: 2001000000000000000000 != 1000000000000000000] test_onAfterAllocateLQTY_newEpoch_VetoToVeto() (gas: 252110)
[FAIL. Reason: assertion failed: 2001000000000000000000 != 1000000000000000000] test_onAfterAllocateLQTY_sameEpoch_NoVetoToVeto() (gas: 355481)
[FAIL. Reason: assertion failed: 2001000000000000000000 != 1000000000000000000] test_onAfterAllocateLQTY_sameEpoch_VetoToNoVeto() (gas: 355460)
[FAIL. Reason: assertion failed: 2001000000000000000000 != 1000000000000000000] test_onAfterAllocateLQTY_sameEpoch_VetoToVeto() (gas: 204028)

I believe they were implemented incorrectly as they were allowing one caller to both vote and veto, but I wanted to make sure.

CC: @jltqy @ColinPlatt

@GalloDaSballo
Copy link
Collaborator Author

I fixed the tests

I believe we should add invariant tests here:

  • A user can only have XOR vote or veto for each instance
  • Sum of user votes is at most their vote power
  • Any user can never reset other users vote power

@GalloDaSballo
Copy link
Collaborator Author

Closing in favour of #13
See my comment there

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.

1 participant