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

audit: NM audit fixes +ISolverHooks refactor #5

Merged
merged 15 commits into from
Jul 11, 2024
Merged

Conversation

parketh
Copy link
Contributor

@parketh parketh commented Jul 8, 2024

Summary

This PR contains:

  1. Fixes from the Nethermind audit
  2. Refactor of the ISolverQuoter -> ISolverHooks interface to allow custom state updates after swaps execute

Audit fixes

  • 44c68c2: Add an assert_market_owner() check to prevent non-owner withdrawals from private vaults. We also add a new test case to prevent future regression of this access control check.
  • f35bbdc, 668ee67: Add new timelock mechanism to address the risk of swap price manipulation via the max_delta params. Contract admin will now be able to set a delay (in seconds) which must pass before queued market parameter updates are set. The delay is skipped if market params are being initialised for the first time.
  • 50613ec: add a range check and emit error message in case of u32 underflow
  • 8da868f: copy validity checks from quote() function to swap()
  • 84cf7ce: add inline documentation to explain custom i32 implementation (no code changes)
  • f676015, 39f0ae7: rename withdraw() -> withdraw_private() and withdraw_at_ratio() -> withdraw_public() for clarity, and add some test cases
  • 5a7e900: remove unnecessary state read
  • 2d9985b: add new check for non-equality between base and quote tokens

Full details documented here.

ISolverHooks refactor

The ISolverQuoter interface provided a read-only quote() function to compute the swap amounts for execution by swap().

However, custom solver implementations may also wish to execute their own state updates after swaps occurs, for example to record new internal values based on the swapped amounts.

To achieve this, we add a new function after_swap() that is called at the end of swap() execution to accomodate custom solver logic. Note that after_swap() is only callable by the contract itself. We add a reentrancy guard to prevent unwanted callers, which is toggled on / off before and after execution.

We also rename the interface ISolverHooks to better reflect its purpose.

Testing

snforge test

@parketh parketh changed the title audit: fixes from NM audit review audit: NM audit fixes + ISolverHooks interface Jul 11, 2024
@parketh parketh changed the title audit: NM audit fixes + ISolverHooks interface audit: NM audit fixes + updates to ISolverHooks interface Jul 11, 2024
@parketh parketh changed the title audit: NM audit fixes + updates to ISolverHooks interface feat: NM audit fixes +ISolverHooks interface refactor Jul 11, 2024
@parketh parketh changed the title feat: NM audit fixes +ISolverHooks interface refactor NM audit fixes +ISolverHooks interface refactor Jul 11, 2024
@parketh parketh changed the title NM audit fixes +ISolverHooks interface refactor NM audit fixes +ISolverHooks refactor Jul 11, 2024
@parketh parketh changed the title NM audit fixes +ISolverHooks refactor audit: NM audit fixes +ISolverHooks refactor Jul 11, 2024
@parketh parketh merged commit ae6ea21 into main Jul 11, 2024
1 check passed
@parketh parketh deleted the audit/nm-audit-fix branch August 21, 2024 08:31
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