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

Remove _getTotalTokens internal getter #1794

Merged
merged 4 commits into from
Sep 19, 2022
Merged

Conversation

TomAFrench
Copy link
Contributor

We actually do length checking of the balances array when upscaling so this check is unnecessary.

@TomAFrench TomAFrench requested a review from nventuro September 19, 2022 15:29
@@ -332,8 +329,6 @@ abstract contract BasePool is
uint256 protocolSwapFeePercentage,
bytes memory userData
) external override returns (uint256 bptOut, uint256[] memory amountsIn) {
InputHelpers.ensureInputLengthMatch(balances.length, _getTotalTokens());
Copy link
Contributor

Choose a reason for hiding this comment

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

This is safe to do because we also don't check the length in eg. onJoinPool. _onJoin and _onExit must implement this check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

However we have must stronger trust in the caller in those cases. If the Vault has fallen out of sync with the Pool then the Pool has acted improperly. Users can provide arbitrary data however.

@TomAFrench TomAFrench merged commit 0c8cd11 into master Sep 19, 2022
@TomAFrench TomAFrench deleted the remove-total-tokens branch September 19, 2022 16:04
TomAFrench added a commit that referenced this pull request Sep 19, 2022
* master:
  Upgrade ignore-warnings (#1789)
  Expose getter for expected compression error on ValueCompression.sol (#1793)
  Remove `_getTotalTokens` internal getter (#1794)
  Remove unused imports (#1795)
  Make base pool better (#1790)
  Update readme.md
  Deploy relayer v4 (#1791)
  [Deployments] Batch relayer V4 preparation. (#1706)
  refactor: remove BaseWeightedPool (#1788)
  Move ManagedPool swap, join, exit logic to ManagedPoolAMMLogic (#1787)
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