Platform: Code4rena
Start Date: 05/09/2023
Pot Size: $50,000 USDC
Total HM: 2
Participants: 16
Period: 6 days
Judge: GalloDaSballo
Total Solo HM: 2
Id: 284
League: ETH
Rank: 5/16
Findings: 1
Award: $586.41
🌟 Selected for report: 1
🚀 Solo Findings: 0
🌟 Selected for report: Sathish9098
Also found by: Baki
586.413 USDC - $586.41
6000 GAS
, 3 SLOT
Each slot saved can avoid an extra Gsset (20000 gas) for the first setting of the struct.
Subsequent reads as well as writes have smaller gas savings.
expiry
can be uint96 instead of uint256
: Saves 2000 GAS
, 1 SLOT
A uint96
can store values from 0 to 2^96 - 1
, which is a very large range. However, it's important to note that Ethereum's block.timestamp
is a Unix timestamp
, which represents time in seconds.
A uint96
would overflow after approximately 2,508,149,904,626,209 years
when storing time in seconds. This is an extremely long time frame, and it's highly unlikely that Ethereum or any blockchain system will remain unchanged for such an extended period.
FILE: 2023-09-delegate/src/libraries/DelegateTokenLib.sol 20: struct DelegateInfo { 21: address principalHolder; 22: IDelegateRegistry.DelegationType tokenType; 23: address delegateHolder; 24: uint256 amount; 25: address tokenContract; + 28: uint96 expiry; 26: uint256 tokenId; 27: bytes32 rights; - 28: uint256 expiry; 29: }
signerSalt
, expiryLength
can be uint128
instead of uint256
: Saves 4000 GAS
, 2 SLOT
In many blockchain protocols, the usage of salt
often involves values within the range of uint32
, as they provide a sufficient numeric space for generating unique salts. Therefore, adopting a uint32 data type for the signerSalt field is a reasonable choice, as it aligns with common practices and conserves storage resources.
For the expiryLength
field, which is intended to store the duration
or length
of an expiration period
, using uint128
is more than adequate. This choice allows for a vast range of possible expiration lengths, accommodating a wide spectrum of use cases without incurring unnecessary storage overhead
FILE: 2023-09-delegate/src/libraries/CreateOffererLib.sol 89: struct Context { 90: bytes32 rights; + 91: uint128 signerSalt; + 92: uint128 expiryLength; - 91: uint256 signerSalt; - 92: uint256 expiryLength; 93: CreateOffererEnums.ExpiryType expiryType; 94: CreateOffererEnums.TargetToken targetToken; 95: } 98: struct Order { 99: bytes32 rights; - 100: uint256 expiryLength; - 101: uint256 signerSalt; + 100: uint128 expiryLength; + 101: uint128 signerSalt; 102: address tokenContract; 103: CreateOffererEnums.ExpiryType expiryType; 104: CreateOffererEnums.TargetToken targetToken; 105: }
20000 GAS
Saves a storage slot. If the variable is assigned a non-zero value, saves Gsset (20000 gas)
. If it's assigned a zero value, saves Gsreset (2900 gas)
. If the variable remains unassigned, there is no gas savings unless the variable is public, in which case the compiler-generated non-payable getter deployment cost is saved. If the state variable is overriding an interface's public function, mark the variable as constant or immutable so that it does not use a storage slot
FILE: delegate-registry/src/DelegateRegistry.sol 18: mapping(bytes32 delegationHash => bytes32[5] delegationStorage) internal delegations;
300 GAS
, 3 SLOD
Caching of a state variable replaces each Gwarmaccess (100 gas) with a cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.
nonce.value
, receivers.targetTokenReceiver
, receivers.fulfiller
should be cached : Saves 300 GAS
, 3 SLOD
FILE: 2023-09-delegate/src/libraries/CreateOffererLib.sol + uint256 value_ = nonce.value ; - 185: if (nonce.value != contractNonce) revert CreateOffererErrors.InvalidContractNonce(nonce.value, contractNonce); + 185: if (value_ != contractNonce) revert CreateOffererErrors.InvalidContractNonce(value_, contractNonce); + address targetTokenReceiver_ = receivers.targetTokenReceiver ; + address fulfiller_ = receivers.fulfiller; 289: //slither-disable-start timestamp 300: if ( keccak256( abi.encode( IDelegateTokenStructs.DelegateInfo({ tokenType: tokenType, - principalHolder: decodedContext.targetToken == CreateOffererEnums.TargetToken.principal ? receivers.targetTokenReceiver : receivers.fulfiller, + principalHolder: decodedContext.targetToken == CreateOffererEnums.TargetToken.principal ? targetTokenReceiver_ : fulfiller_ , - delegateHolder: decodedContext.targetToken == CreateOffererEnums.TargetToken.delegate ? receivers.targetTokenReceiver : receivers.fulfiller, + delegateHolder: decodedContext.targetToken == CreateOffererEnums.TargetToken.delegate ? targetTokenReceiver_ : fulfiller_ , expiry: CreateOffererHelpers.calculateExpiry(decodedContext.expiryType, decodedContext.expiryLength), rights: decodedContext.rights, tokenContract: consideration.token, tokenId: (tokenType != IDelegateRegistry.DelegationType.ERC20) ? consideration.identifier : 0, amount: (tokenType != IDelegateRegistry.DelegationType.ERC721) ? consideration.amount : 0 }) ) ) != keccak256(abi.encode(IDelegateToken(delegateToken).getDelegateInfo(DelegateTokenHelpers.delegateIdNoRevert(address(this), identifier)))) ) revert CreateOffererErrors.DelegateInfoInvariant();
2450 GAS
for every iteration from 7 instances
Assembly is more gas efficient for loops. Saves minimum 350 GAS
per iteration as per remix gas checks.
FILE: Breadcrumbsdelegate-registry/src/DelegateRegistry.sol - 35: for (uint256 i = 0; i < data.length; ++i) { - //slither-disable-next-line calls-loop,delegatecall-loop - (success, results[i]) = address(this).delegatecall(data[i]); - if (!success) revert MulticallFailed(); - } + assembly { + // Load the length of the data array + let dataSize := mload(data) + + // Initialize the results array + let results := mload(0x40) + mstore(results, dataSize) + + // Initialize a counter (i) to zero + let i := 0 + + for { } lt(i, dataSize) { } { + // Start loop + + // Load the next calldata from the data array + let calldataPtr := add(add(data, 0x20), mul(i, 0x20)) + let calldataSize := mload(calldataPtr) + + // Perform delegatecall + let success := delegatecall(gas(), address(), add(calldataPtr, 0x04), calldataSize, 0, 0) + + // Store the result and check for success + if iszero(success) { + // Revert if delegatecall fails + revert(0, 0) + } + mstore(add(results, mul(i, 0x20)), success) + + // Increment the counter + i := add(i, 1) + + // End loop + } + + // results contains the success status of each delegatecall + + } 275: for (uint256 i = 0; i < hashes.length; ++i) { 312: for (uint256 i = 0; i < length; ++i) { tempLocation = locations[i]; assembly { tempValue := sload(tempLocation) } contents[i] = tempValue; } 386: for (uint256 i = 0; i < hashesLength; ++i) { hash = hashes[i]; if (_invalidFrom(_loadFrom(Hashes.location(hash)))) continue; filteredHashes[count++] = hash; } 393: for (uint256 i = 0; i < count; ++i) { hash = filteredHashes[i]; location = Hashes.location(hash); (address from, address to, address contract_) = _loadDelegationAddresses(location); delegations_[i] = Delegation({ type_: Hashes.decodeType(hash), to: to, from: from, rights: _loadDelegationBytes32(location, Storage.POSITIONS_RIGHTS), amount: _loadDelegationUint(location, Storage.POSITIONS_AMOUNT), contract_: contract_, tokenId: _loadDelegationUint(location, Storage.POSITIONS_TOKEN_ID) }); } 417: for (uint256 i = 0; i < hashesLength; ++i) { hash = hashes[i]; if (_invalidFrom(_loadFrom(Hashes.location(hash)))) continue; filteredHashes[count++] = hash; } 423: for (uint256 i = 0; i < count; ++i) { validHashes[i] = filteredHashes[i]; }
Saves 13 GAS
for local variable and 2000 GAS
for state variable
FILE: Breadcrumbsdelegate-registry/src/DelegateRegistry.sol 35: for (uint256 i = 0; i < data.length; ++i) { 275: for (uint256 i = 0; i < hashes.length; ++i) { 312: for (uint256 i = 0; i < length; ++i) { 386: for (uint256 i = 0; i < hashesLength; ++i) { 393: for (uint256 i = 0; i < count; ++i) { 417: for (uint256 i = 0; i < hashesLength; ++i) { 423: for (uint256 i = 0; i < count; ++i) {
FILE: delegate-registry/src/DelegateRegistry.sol 213: ? type(uint256).max 215: if (!Ops.or(rights == "", amount == type(uint256).max)) { 217: ? type(uint256).max 232: ? type(uint256).max 234: if (!Ops.or(rights == "", amount == type(uint256).max)) { 236: ? type(uint256).max 372: uint256 cleanUpper12Bytes = type(uint256).max << 160;
File: delegate-registry/src/DelegateRegistry.sol 56: } else if (loadedFrom == msg.sender) { 75: } else if (loadedFrom == msg.sender) { 85: } else if (loadedFrom == msg.sender) { 115: } else if (loadedFrom == msg.sender) { 118: } else if (loadedFrom == msg.sender) { 140: } else if (loadedFrom == msg.sender) { 143: } else if (loadedFrom == msg.sender) {
Per iterations saves 26 GAS
FILE: delegate-registry/src/DelegateRegistry.sol + bytes32 location ; + address from ; for (uint256 i = 0; i < hashes.length; ++i) { - bytes32 location = Hashes.location(hashes[i]); - address from = _loadFrom(location); + location = Hashes.location(hashes[i]); + from = _loadFrom(location); if (_invalidFrom(from)) { delegations_[i] = Delegation({type_: DelegationType.NONE, to: address(0), from: address(0), rights: "", amount: 0, contract_: address(0), tokenId: 0}); } else { (, address to, address contract_) = _loadDelegationAddresses(location); delegations_[i] = Delegation({ type_: Hashes.decodeType(hashes[i]), to: to, from: from, rights: _loadDelegationBytes32(location, Storage.POSITIONS_RIGHTS), amount: _loadDelegationUint(location, Storage.POSITIONS_AMOUNT), contract_: contract_, tokenId: _loadDelegationUint(location, Storage.POSITIONS_TOKEN_ID) });
#0 - GalloDaSballo
2023-10-02T09:05:46Z
Very good work with the first 2 findings!
#1 - c4-judge
2023-10-02T09:08:45Z
GalloDaSballo marked the issue as grade-a
#2 - c4-judge
2023-10-02T09:08:53Z
GalloDaSballo marked the issue as selected for report
#3 - 0xfoobar
2023-10-03T23:34:18Z
Useful findings on the gas stuff, not sure the refactoring is worth the loss of struct ordering here but will think it over
#4 - c4-sponsor
2023-10-03T23:34:24Z
0xfoobar (sponsor) confirmed
#5 - sathishpic22
2023-10-05T12:33:51Z
Hi @GalloDaSballo
https://github.com/code-423n4/2023-09-delegate-findings/issues/380 this issue is disputed by sponsor . Still Grade A ?