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.

Sign up for GitHub

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

Jump to bottom

Refactor SafeTransferLib and interfaces #343

Merged
merged 3 commits into from
Aug 17, 2023
Merged

Refactor SafeTransferLib and interfaces #343

merged 3 commits into from
Aug 17, 2023

Conversation

QGarchery
Copy link
Contributor

@QGarchery QGarchery commented Aug 17, 2023 •
edited
Loading

This PR does 2 refactors:

  • removing functions in the IERC20 interface to ensure that the transfer and transferFrom functions are not called directly (I had a bug because of this mistake in the Certora development, and we had one earlier on the main files)
  • removing the code length check on the safe transfer lib. This is described in this thread, but basically it's already possible to create buggy markets, and it's up to the market creator to ensure that the tokens are behaving correctly. Even with this check, it's still possible to have a token contract that cheats or even selfdestruct and reappear afterwards (with the CREATE, CREATE2 and SELFDESTRUCT hack)

@QGarchery QGarchery self-assigned this Aug 17, 2023
@QGarchery QGarchery requested review from a team, Rubilmax, MerlinEgalite, pakim249CAL, Jean-Grimal, makcandrov, peyha and MathisGD and removed request for a team August 17, 2023 09:58
@QGarchery QGarchery requested a review from MathisGD August 17, 2023 16:39
@MerlinEgalite MerlinEgalite merged commit 71eb4d7 into main Aug 17, 2023
@MerlinEgalite MerlinEgalite deleted the refactor/transfer-lib-interface branch August 17, 2023 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MathisGD MathisGD MathisGD approved these changes

@pakim249CAL pakim249CAL pakim249CAL approved these changes

@MerlinEgalite MerlinEgalite MerlinEgalite approved these changes

@Rubilmax Rubilmax Awaiting requested review from Rubilmax Rubilmax was automatically assigned from morpho-labs/onchain

@Jean-Grimal Jean-Grimal Awaiting requested review from Jean-Grimal Jean-Grimal was automatically assigned from morpho-labs/onchain

@makcandrov makcandrov Awaiting requested review from makcandrov makcandrov was automatically assigned from morpho-labs/onchain

@peyha peyha Awaiting requested review from peyha peyha was automatically assigned from morpho-labs/protocol

Assignees

@QGarchery QGarchery

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@QGarchery @MerlinEgalite @pakim249CAL @MathisGD