Skip to content

Fix ERC1967Upgrade.huff offset bug and incorrect delegatecall params #147

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

Open
kadenzipfel wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix ERC1967Upgrade.huff offset bug and incorrect delegatecall params #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d9a147
Add proper upgradeToAndCall tests
kadenzipfel Jul 3, 2026
65a7222
Fix incorrect byte offset
kadenzipfel Jul 3, 2026
cb72c01
Fix incorrect delegatecall params
kadenzipfel Jul 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions src/proxies/ERC1967Upgrade.huff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,21 +90,21 @@
// Copy the data from calldata to memory
swap2 not // [!forceCall, data, address]

dup2 calldataload dup1 // [dataSize, dataSize, forceCall, data, address]
dup2 0x04 add calldataload dup1 // [dataSize, dataSize, !forceCall, data, address]
swap2 swap1 iszero and // [(!forceCall && dataSize == 0), dataSize, data, address]
__IgnoreCall_And_Clean_Stack jumpi

dup1 swap2 0x20 add // [&data[0], dataSize, dataSize, address]
dup1 swap2 0x24 add // [&data[0] (= data + 0x04 + 0x20), dataSize, dataSize, address]
0x00 // [destOffset, calldataOffset, dataSize, dataSize, address]
calldatacopy // [dataSize, address]

// Call the implementation with the given data
0x00 // [retOffset, dataSize, address]
swap2 // [argSize, retOffset, address]
0x00 // [argOffset, argSize, retOffset, address]
0x00 // [retSize, argOffset, argSize, retOffset, address]
swap4 // [address, argOffset, argSize, retOffset, retSize]
gas // [gas, address, argOffset, argSize, retOffset, retSize]
// delegatecall(gas, addr=address, argOffset=0, argSize=dataSize, retOffset=0, retSize=0)
0x00 0x00 0x00 // [retOffset, argOffset, retSize, argSize, addr]
swap3 // [argSize, argOffset, retSize, retOffset, addr]
swap2 // [retSize, argOffset, argSize, retOffset, addr]
swap4 // [addr, argOffset, argSize, retOffset, retSize]
gas // [gas, addr, argOffset, argSize, retOffset, retSize]
delegatecall // [success]
__DelegateCall_Success jumpi // []
0x00 dup1 revert
Expand Down Expand Up @@ -267,7 +267,7 @@
0x00 0x00 log2 // [newBeacon, data, forceCall]

// Copy the calldata to memory
dup2 calldataload dup1 // [dataSize, dataSize, newBeacon, data, forceCall]
dup2 0x04 add calldataload dup1 // [dataSize, dataSize, newBeacon, data, forceCall]
swap4 not swap1 iszero and // [(!forceCall && dataSize == 0), newBeacon, data, dataSize]
__IgnoreCall_And_Clean_Stack jumpi

Expand All @@ -287,7 +287,7 @@
// Delegate call to the implementation address
0x00 mload // [implementation, newBeacon, data, dataSize]
swap3 swap1 swap2 // [data, dataSize, newBeacon, implementation]
dup2 dup2 0x20 add // [&data[0], dataSize, data, dataSize, newBeacon, implementation]
dup2 dup2 0x24 add // [&data[0] (= data + 0x04 + 0x20), dataSize, data, dataSize, newBeacon, implementation]
0x00 // [destOffset, calldataOffset, dataSize, data, dataSize, newBeacon, implementation]
calldatacopy // [data, dataSize, newBeacon, implementation]
swap1 // [dataSize, data, newBeacon, implementation]
Expand Down
34 changes: 33 additions & 1 deletion test/proxies/ERC1967Proxy.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,39 @@ contract ProxiesTest is Test, ERC1155Recipient {

proxy.upgradeToAndCallUUPS(address(proxiableUUID), bytes(mintCalldata), false);
assertEq(proxy.implementation(), address(proxiableUUID));
}
}

function testUpgradeToAndCallExecutesInitCall() public {
bytes memory mintCalldata = abi.encodeWithSignature(
"mint(address,uint256,uint256,bytes)",
address(this), uint256(1), uint256(7), bytes("")
);
proxy.upgradeToAndCall(address(implementation), mintCalldata, false);
assertEq(MockERC1155(address(proxy)).balanceOf(address(this), 1), 7);
}

function testUpgradeToAndCallUUPSExecutesInitCall() public {
bytes memory mintCalldata = abi.encodeWithSignature(
"mint(address,uint256,uint256,bytes)",
address(this), uint256(2), uint256(9), bytes("")
);
proxy.upgradeToAndCallUUPS(address(proxiableUUID), mintCalldata, false);
assertEq(MockERC1155(address(proxy)).balanceOf(address(this), 2), 9);
}

function testUpgradeBeaconAndCallExecutesInitCall() public {
// The beacon path only sets the beacon slot; point the implementation slot at the same
// logic (MockERC1155) so state written by the beacon's init call can be read back through
// the proxy's delegatecall fallback (which routes via the implementation slot).
proxy.upgradeTo(address(implementation));

bytes memory mintCalldata = abi.encodeWithSignature(
"mint(address,uint256,uint256,bytes)",
address(this), uint256(3), uint256(11), bytes("")
);
proxy.upgradeBeaconAndCall(address(beacon), mintCalldata, false);
assertEq(MockERC1155(address(proxy)).balanceOf(address(this), 3), 11);
}


function testProxyPassthrough() public {
Expand Down
Loading