OpenSea Seaport 1.2 contest - saneryee's results

A marketplace protocol for safely and efficiently buying and selling NFTs.

General Information

Platform: Code4rena

Start Date: 13/01/2023

Pot Size: $100,500 USDC

Total HM: 1

Participants: 23

Period: 10 days

Judge: hickuphh3

Total Solo HM: 1

Id: 201

League: ETH

OpenSea

Findings Distribution

Researcher Performance

Rank: 11/23

Findings: 1

Award: $169.76

Gas:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: Dravee

Also found by: 0xSmartContract, IllIllI, RaymondFam, Rolezn, atharvasama, c3phas, karanctf, saneryee

Labels

bug
G (Gas Optimization)
grade-b
G-08

Awards

169.7607 USDC - $169.76

External Links

Gas Optimizations Report

IssueInstances
[G-001]x += y or x -= y costs more gas than x = x + y or x = x - y for state variables25
[G-002]storage pointer to a structure is cheaper than copying each value of the structure into memory, same for array and mapping3
[G-003]Functions guaranteed to revert when called by normal users can be marked payable3
[G-004]internal functions only called once can be inlined to save gas6
[G-005]Replace modifier with function1

[G-001] x += y or x -= y costs more gas than x = x + y or x = x - y for state variables

Impact

Using the addition operator instead of plus-equals saves 113 gas. Usually does not work with struct and mappings.

Findings

Total:25

contracts/lib/ConsiderationDecoder.sol#L399

399:    for (uint256 offset = 0; offset < tailOffset; offset += OneWord) {

contracts/lib/ConsiderationDecoder.sol#L498

498:    for (uint256 offset = 0; offset < tailOffset; offset += OneWord) {

contracts/lib/ConsiderationDecoder.sol#L538

538:    for (uint256 offset = 0; offset < tailOffset; offset += OneWord) {

contracts/lib/ConsiderationDecoder.sol#L630

630:    for (uint256 offset = 0; offset < tailOffset; offset += OneWord) {

contracts/lib/ConsiderationDecoder.sol#L673

673:    for (uint256 offset = 0; offset < tailOffset; offset += OneWord) {

contracts/lib/ConsiderationDecoder.sol#L744

744:    for (uint256 offset = 0; offset < tailOffset; offset += OneWord) {

contracts/lib/BasicOrderFulfiller.sol#L965

965:    nativeTokensRemaining -= additionalRecipientAmount;

contracts/lib/BasicOrderFulfiller.sol#L1097

1097:    amount -= additionalRecipientAmount;

contracts/lib/BasicOrderFulfiller.sol#L1112

1112:    i += AdditionalRecipient_size;

contracts/lib/BasicOrderFulfiller.sol#L941

941:    i += AdditionalRecipient_size

contracts/lib/OrderCombiner.sol#L230

230:    for (uint256 i = 32; i < terminalMemoryOffset; i += 32) {

contracts/lib/OrderCombiner.sol#L451

451:    for (uint256 i = 32; i < terminalMemoryOffset; i += 32) {

contracts/lib/ConsiderationEncoder.sol#L259

259:    tailOffset += considerationSize;

contracts/lib/ConsiderationEncoder.sol#L400

400:    tailOffset += considerationSize;

contracts/lib/ConsiderationEncoder.sol#L172

172:    tailOffset += contextSize;

contracts/lib/ConsiderationEncoder.sol#L273

273:    tailOffset += contextSize;

contracts/lib/ConsiderationEncoder.sol#L413

413:    tailOffset += extraDataSize;

contracts/lib/ConsiderationEncoder.sol#L287

287:    tailOffset += orderHashesSize;

contracts/lib/ConsiderationEncoder.sol#L429

429:    tailOffset += orderHashesSize;

contracts/lib/ConsiderationEncoder.sol#L240

240:    tailOffset += offerSize;

contracts/lib/ConsiderationEncoder.sol#L379

379:    tailOffset += offerSize;

contracts/lib/ConsiderationEncoder.sol#L523

523:    tailOffset += OneWord;

contracts/lib/ConsiderationEncoder.sol#L155

155:    tailOffset += maximumSpentSize;

contracts/lib/ConsiderationEncoder.sol#L134

134:    tailOffset += minimumReceivedSize;

contracts/lib/ConsiderationEncoder.sol#L514

514:    tailOffset += offerAndConsiderationSize;

Recommendation

Same as description

[G-002] storage pointer to a structure is cheaper than copying each value of the structure into memory, same for array and mapping

Impact

It may not be obvious, but every time you copy a storage struct/array/mapping to a memory variable, you are literally copying each member by reading it from storage, which is expensive. And when you use the storage keyword, you are just storing a pointer to the storage, which is much cheaper.

Findings

Total:3

contracts/lib/OrderCombiner.sol#L685

685:    bool[] memory availableOrders = new bool[](totalOrders);

contracts/lib/OrderFulfiller.sol#L116

116:    bytes32[] memory orderHashes = new bytes32[](1);

contracts/lib/TypehashDirectory.sol#L32

32:    bytes32[] memory typeHashes = new bytes32[](MaxTreeHeight);

Recommendation

Using storage instead of memory

[G-003] Functions guaranteed to revert when called by normal users can be marked payable

Impact

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost.

Findings

Total:3

contracts/conduit/Conduit.sol#L94

94:    ) external override onlyOpenChannel returns (bytes4 magicValue) {

contracts/conduit/Conduit.sol#L129

129:    ) external override onlyOpenChannel returns (bytes4 magicValue) {

contracts/conduit/Conduit.sol#L157

157:    ) external override onlyOpenChannel returns (bytes4 magicValue) {

Recommendation

Mark the function as payable.

[G-004] internal functions only called once can be inlined to save gas

Impact

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

Findings

Total:6

contracts/lib/OrderCombiner.sol#L867

867:    function _emitOrdersMatched(bytes32[] memory orderHashes) internal {

contracts/lib/Executor.sol#L454

454:    function _triggerIfArmed(bytes memory accumulator) internal {

contracts/lib/Executor.sol#L479

479:    function _trigger(bytes32 conduitKey, bytes memory accumulator) internal {

contracts/lib/GettersAndDerivers.sol#L295

295:    function _domainSeparator() internal view returns (bytes32) {

contracts/lib/ConsiderationBase.sol#L161

161:    function _nameString() internal pure virtual returns (string memory) {

contracts/lib/ReentrancyGuard.sol#L62

62:    function _assertNonReentrant() internal view {

[G-005] Replace modifier with function

Impact

Modifiers make code more elegant, but cost more than normal functions.

Findings

Total:1

contracts/conduit/Conduit.sol#L40

40:    modifier onlyOpenChannel() {

#0 - c4-judge

2023-01-26T03:34:18Z

HickupHH3 marked the issue as grade-b

#1 - HickupHH3

2023-01-26T06:24:45Z

[G-002] is invalid, instantiating new array, not copying. [G-004] and [G-005] are similar.

#2 - HickupHH3

2023-01-26T06:24:58Z

Very borderline satisfactory.

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter