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

[EHF-01C] Inefficient mapping Lookups #734

Closed
zajck opened this issue Aug 1, 2023 · 0 comments · Fixed by #752
Closed

[EHF-01C] Inefficient mapping Lookups #734

zajck opened this issue Aug 1, 2023 · 0 comments · Fixed by #752
Assignees

Comments

@zajck
Copy link
Member

zajck commented Aug 1, 2023

EHF-01C: Inefficient mapping Lookups

Type Severity Location
Gas Optimization ExchangeHandlerFacet.sol:L993, L1001, L1005, L1013

Description:

The linked statements perform key-based lookup operations on mapping declarations from storage multiple times for the same key redundantly.

Example:

if (_condition.method == EvaluationMethod.SpecificToken) {
    // How many times has this token id been used to commit to offers in the group?
    uint256 commitCount = lookups.conditionalCommitsByTokenId[_tokenId][_groupId];

    require(commitCount < _condition.maxCommits, MAX_COMMITS_TOKEN_REACHED);

    allow = holdsSpecificToken(_buyer, _condition, _tokenId);

    if (allow) {
        // Increment number of commits to the group for this token id if they are allowed to commit
        lookups.conditionalCommitsByTokenId[_tokenId][_groupId] = ++commitCount;
    }
} else if (_condition.method == EvaluationMethod.Threshold) {
    // How many times has this address committed to offers in the group?
    uint256 commitCount = lookups.conditionalCommitsByAddress[_buyer][_groupId];

    require(commitCount < _condition.maxCommits, MAX_COMMITS_ADDRESS_REACHED);

    allow = holdsThreshold(_buyer, _condition, _tokenId);

    if (allow) {
        // Increment number of commits to the group for this address if they are allowed to commit
        lookups.conditionalCommitsByAddress[_buyer][_groupId] = ++commitCount;
    }
} else {

Recommendation:

As the lookups internally perform an expensive keccak256 operation, we advise the lookups to be cached wherever possible to a single local declaration that either holds the value of the mapping in case of primitive types or holds a storage pointer to the struct contained.

@zajck zajck self-assigned this Aug 4, 2023
zajck added a commit that referenced this issue Aug 4, 2023
anajuliabit added a commit that referenced this issue Aug 14, 2023
Co-authored-by: Mischa <mischa@mmt.me.uk>
Co-authored-by: Ana Julia Bittencourt <anajuliabit@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant