Skip to content

Conversation

@prestwich
Copy link
Member

@prestwich prestwich commented Dec 2, 2025

Add an L1 contract for Signet users

Copy link
Member Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@prestwich prestwich marked this pull request as ready for review December 2, 2025 13:31
import {PecorinoConstants} from "../chains/Pecorino.sol";

abstract contract SignetL1 {
/// @notice Sentinal value for the native asset in order inputs/outputs
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// @notice Sentinal value for the native asset in order inputs/outputs
/// @notice Sentinel value for the native asset in order inputs/outputs

bool is7702;
assembly {
let ptr := mload(0x40)
extcodecopy(caller(), ptr, 0, 0x20)
Copy link
Member

Choose a reason for hiding this comment

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

this is kind of obscure, but isn't this a bug under a certain circumstance?

we check above for the correct code length on 7702 delegated addresses, but do the codecopy for prefix checking on caller, which IIRC, returns who is the call sender. So for any 7702 delegated user who uses an intermediary contract (e.g some router/defi thing) to call this, we'll return an aliased address instead of the EOA. shouldn't it be address()?

ethToSignet(amount);
return;
}
IERC20(token).approve(address(PASSAGE), amount);
Copy link
Member

Choose a reason for hiding this comment

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

i think this might be problematic w/ USDT since they're non ERC20 compliant, and solidity tries to decode return data even if you don't use it. so this might straight up revert. we should use SafeERC20 here

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