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

build: router #16

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

build: router #16

wants to merge 5 commits into from

Conversation

Schlagonia
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@fp-crypto fp-crypto left a comment

Choose a reason for hiding this comment

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

Seems pretty straightforward.

@@ -0,0 +1,34 @@
// forked from https://github.com/Uniswap/v3-periphery/blob/main/contracts/base/Multicall.sol
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this an exact copy?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

think so

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

guess i removed the interface andd added the comment to the function

}
}

abstract contract IWETH9 is ERC20 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this an abstract contract instead of an interface?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Already had the ERC20 dependency i guess

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FWIW these two contracts were just basically copied from the 4626 Router that was forked from FEI https://github.com/yearn/Yearn-ERC4626-Router/tree/master/src/external


import {IPermit2} from "./IPermit2.sol";

contract STBRouter is Multicall, PeripheryPayments {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm feeling dumb ... isn't PeripheryPayments completely open to all callers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes. Any tokens left in the router after a txn can get pulled by anyone.

The idea is the multicall allows any of them to be used through a delegatecall

Choose a reason for hiding this comment

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

do you know if the rationale for Uniswap was to not keep internal accounting, and thus simplify the implementation?

src/router/STBRouter.sol Outdated Show resolved Hide resolved

import {IPermit2} from "./IPermit2.sol";

contract STBRouter is Multicall, PeripheryPayments {

Choose a reason for hiding this comment

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

do you know if the rationale for Uniswap was to not keep internal accounting, and thus simplify the implementation?

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.

3 participants