Skip to content

feat: improve gas of TransferHelper - #105

Merged
marktoda merged 2 commits into
take-safefrom
take-safe-better-gas
Jun 5, 2022
Merged

marktoda merged 2 commits into
take-safefrom
take-safe-better-gas

Conversation

@marktoda

@marktoda marktoda commented Jun 3, 2022

Copy link
Copy Markdown
Contributor

This commit uses the solmate SafeTransfer library implementation
which has much improved gas performance over the existing
implementation. See https://github.com/Rari-Capital/solmate/blob/main/src/utils/SafeTransferLib.sol#L63

We copy the implementation in rather than import for typechecking as the
somlate library uses their custom ERC20 implementation and I didn't want
to have add ugly casts everywhere

@marktoda
marktoda force-pushed the take-safe-better-gas branch from 404a17b to d93c655 Compare June 3, 2022 15:53
This commit uses the solmate SafeTransfer library implementation
which has much improved gas performance over the existing
implementation. See https://github.com/Rari-Capital/solmate/blob/main/src/utils/SafeTransferLib.sol#L63

We copy the implementation in rather than import for typechecking as the
somlate library uses their custom ERC20 implementation and I didn't want
to have add ugly casts everywhere
@marktoda
marktoda force-pushed the take-safe-better-gas branch from d93c655 to 09d443b Compare June 3, 2022 15:54
@ewilz

ewilz commented Jun 3, 2022

Copy link
Copy Markdown
Contributor

LGTM. We do not really have a merge process on this repo (usually there's been a repo owner of sorts) Maybe something to discuss at the HUDDLE

@willpote

willpote commented Jun 5, 2022

Copy link
Copy Markdown
Contributor

LGTM. We do not really have a merge process on this repo (usually there's been a repo owner of sorts) Maybe something to discuss at the HUDDLE

Agree we need a merge/review process, though I do think we should try and avoid having single owners for repos to prevent bottlenecks. +1 for huddle topic

@marktoda
marktoda merged commit 3be78c6 into take-safe Jun 5, 2022
@marktoda
marktoda deleted the take-safe-better-gas branch June 5, 2022 22:18
willpote added a commit that referenced this pull request Jun 5, 2022
* Update package.json files

* Start

* Fix code style issues with Prettier

* feat: improve gas of TransferHelper (#105)

* feat: improve gas of TransferHelper

This commit uses the solmate SafeTransfer library implementation
which has much improved gas performance over the existing
implementation. See https://github.com/Rari-Capital/solmate/blob/main/src/utils/SafeTransferLib.sol#L63

We copy the implementation in rather than import for typechecking as the
somlate library uses their custom ERC20 implementation and I didn't want
to have add ugly casts everywhere

* Fix code style issues with Prettier

Co-authored-by: Lint Action <lint-action@samuelmeuli.com>

Co-authored-by: Lint Action <lint-action@samuelmeuli.com>
Co-authored-by: marktoda <40770586+marktoda@users.noreply.github.com>
Jun1on pushed a commit to Jun1on/v4-core that referenced this pull request Jul 29, 2026
* Update package.json files

* Start

* Fix code style issues with Prettier

* feat: improve gas of TransferHelper (Uniswap#105)

* feat: improve gas of TransferHelper

This commit uses the solmate SafeTransfer library implementation
which has much improved gas performance over the existing
implementation. See https://github.com/Rari-Capital/solmate/blob/main/src/utils/SafeTransferLib.sol#L63

We copy the implementation in rather than import for typechecking as the
somlate library uses their custom ERC20 implementation and I didn't want
to have add ugly casts everywhere

* Fix code style issues with Prettier

Co-authored-by: Lint Action <lint-action@samuelmeuli.com>

Co-authored-by: Lint Action <lint-action@samuelmeuli.com>
Co-authored-by: marktoda <40770586+marktoda@users.noreply.github.com>
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.

4 participants