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

fix(settlement): adds sanity check at end of settlement #278

Merged
merged 2 commits into from
Mar 13, 2023

Conversation

Alexangelj
Copy link
Contributor

MUST MERGE chore/fmt FIRST.

Description

This is not the most efficient function because it does three loops: loop over to get payments, loop over to do payments, loop over all tokens to make sure they were settled.

This can be all done in a single loop, however, this introduces a possible attack vector via a malicious token that executes arbitrary logic in the transferFrom which manipulates the settlement status of some of the other tokens. In a single loop, the check for the net balance would happen in between transferFrom. If there was a way to take tokens out of the contract after they have been transferred in, then there would be a way to get a negative net balance even after the sanity check after the transfer from. That is pretty unlikely though.

Base automatically changed from chore/fmt to main March 13, 2023 15:47
@Alexangelj Alexangelj marked this pull request as ready for review March 13, 2023 15:48
@Alexangelj Alexangelj merged commit b7777da into main Mar 13, 2023
@Alexangelj Alexangelj deleted the fix/tokens-transfer-fee branch March 13, 2023 15:49
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.

Investigate: Tokens with fee on transfer, unsupported
1 participant