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

Make approveVault payable #2067

Merged
merged 1 commit into from
Nov 25, 2022
Merged

Make approveVault payable #2067

merged 1 commit into from
Nov 25, 2022

Conversation

jubeira
Copy link
Contributor

@jubeira jubeira commented Nov 25, 2022

Description

Makes approveVault payable in BaseRelayerLibrary.
I've inspected the remaining public / external non-view functions in BatchRelayerLibrary and they are all payable.

Since we don't have slither yet, we could add tests that check that sending value to those functions doesn't revert, although it's not always straightforward when there are permissions involved. Feedback on this point is welcome; adding a simple test as reference.

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Dependency changes
  • Code refactor / cleanup
  • Documentation or wording changes
  • Other

Checklist:

  • The diff is legible and has no extraneous changes
  • Complex code has been commented, including external interfaces
  • Tests are included for all code paths
  • The base branch is either master, or there's a description of how to merge

Issue Resolution

Closes #1940.

@jubeira jubeira marked this pull request as ready for review November 25, 2022 16:51
@jubeira jubeira merged commit 5db1cdb into master Nov 25, 2022
@jubeira jubeira deleted the relayer-v5/vaultapprove-payable branch November 25, 2022 16:52
TomAFrench added a commit that referenced this pull request Nov 28, 2022
* master: (26 commits)
  Deprecate managed pool deployment (#2044)
  Add support for cash and managed to MockVault (#2064)
  chore: add .nvmrc file (#2066)
  Make `approveVault` payable (`BaseRelayerLibrary`). (#2067)
  Do new aave linear deploy (#2055)
  Deploy entrypoint (#2062)
  (Re) deployment preparation: `AuthorizerAdaptorEntrypoint` (#2060)
  chore: update deployment readmes to point to artifact files rather than abis (#2061)
  Expose deployment artifacts from deployments package rather than ABI + bytecode (#2058)
  chore: add missing action-ids (#2025)
  Remove Protocol Fee Splitter task (#2054)
  Adaptor entrypoint events (#2051)
  Verify AaveLinearPool on deployment (#2040)
  Rename IPoolVersion --> IFactoryCreatedPoolVersion. (#2045)
  docs: remove deprecated networks from jsdoc (#2029)
  Add Version unit test. (#2038)
  Update linear build info (#2041)
  Make unnecessarily async functions sync (#2030)
  Do deployment (#2035)
  Aave linear pool versioning (#2032)
  ...
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.

Make vaultApprove payable
2 participants