0

I have a large contract and I am in the process of splitting it out into two. The goal is to have the functions that are common (and will be used by many other contracts) to be separated out for efficiency.

One of these functions compares items in arrays "ownedSymbols" and "targetAssets". It produces a list "sellSymbols" if any item in "ownedSymbols" is not in "targetAssets".

The below code works fine while "sellSymbols" is stored as a string. As this function will become common, I need it to run in memory so the results aren't confused by calls from different contracts.

pragma solidity >0.8.0;

contract compareArrays {
    string[] public ownedSymbols = ["A","B","C"];
    string[] public targetAssets = ["A","B"];
    string[] sellSymbols;

    event sellListEvent(string[]);

    function sellList(string[] memory _ownedSymbols, string[] memory _targetAssetsList) internal {
        sellSymbols = _ownedSymbols;
        for (uint256 i = 0; i < _targetAssetsList.length; i++) {
            for (uint256 x = 0; x < sellSymbols.length; x++) {
                if (
                    keccak256(abi.encodePacked((sellSymbols[x]))) ==
                    keccak256(abi.encodePacked((_targetAssetsList[i])))
                ) {
                    if (x < sellSymbols.length) {
                        sellSymbols[x] = sellSymbols[sellSymbols.length - 1];
                        sellSymbols.pop();
                    } else {
                        delete sellSymbols;
                    }
                }
            }
        }
        emit sellListEvent(sellSymbols);
    }

    function runSellList() public {
        sellList(ownedSymbols,targetAssets);
    }

}

Ideally the function would run with "string[] memory sellSymbols", however this kicks back an error.

pragma solidity >0.8.0;

contract compareArrays {
    string[] public ownedSymbols = ["A","B","C"];
    string[] public targetAssets = ["A","B"];

    event sellListEvent(string[]);

    function sellList(string[] memory _ownedSymbols, string[] memory _targetAssetsList) internal {
        string[] memory sellSymbols = _ownedSymbols;
        for (uint256 i = 0; i < _targetAssetsList.length; i++) {
            for (uint256 x = 0; x < sellSymbols.length; x++) {
                if (
                    keccak256(abi.encodePacked((sellSymbols[x]))) ==
                    keccak256(abi.encodePacked((_targetAssetsList[i])))
                ) {
                    if (x < sellSymbols.length) {
                        sellSymbols[x] = sellSymbols[sellSymbols.length - 1];
                        sellSymbols.pop();
                    } else {
                        delete sellSymbols;
                    }
                }
            }
        }
        emit sellListEvent(sellSymbols);
    }

    function runSellList() public {
        sellList(ownedSymbols,targetAssets);
    }

}

The error:

TypeError: Member "pop" is not available in string memory[] memory outside of storage.
--> contracts/sellSymbols.sol:20:25:
|
20 | sellSymbols.pop();
| ^^^^^^^^^^^^^^^

Two questions from me:

  1. Is there a way to do this in memory so that the function can be common (i.e. used by multiple contracts at the same time)?
  2. Is there a better way? The below is expensive to run, but it is the only way I have been able to achieve it.

One final comment - I know this would be much easier/cheaper to run off chain. That is not something I am willing to consider as I want this project to be decentralized.

mattblack
  • 1,370
  • 3
  • 13
  • 19

1 Answers1

0

If you want to keep the existing system, the best solution is described here: https://stackoverflow.com/a/49054593/11628256

                    if (x < sellSymbols.length) {
                        sellSymbols[x] = sellSymbols[sellSymbols.length - 1];
                        delete sellSymbols[myArray.length - 1];
                        sellSymbols.length--;
                    } else {
                        delete sellSymbols;
                    }

If all you care about is the presence or absence of a particular asset (and not enumerating through them), what you're going to want to do to really reduce gas costs is something called "lazy evaluation." Lazy evaluation is when instead of computing all results at once (like increasing all balances by 50% by iterating over an array), you modify the getters so that their return value reflects the operation (such as multiplying an internal variable by 50% and multiplying the original result of getBalance by that variable).

So, if this is the case, what you want to do is use the following function instead:

function except(string _item, mapping(string => bool) _ownedSymbols, mapping(string => bool) _targetAssets) internal returns (bool) {
    return _ownedSymbols[_item] && !_targetAssets[_item];
}

<pet peeve>

Finally, I know you say you want this to be decentralized, but I really do feel the urge to say this. If this is a system that doesn't need to be decentralized, don't decentralize it! Decentralization is great for projects that other people rely on - for example, DNS or any sort of token.

From your variable names, it seems that this is probably some sort of system similar to a trading bot. Therefore, the incentive is on you to keep it running, as you are the one that gets the benefits. None of the problems that decentralization solves (censorship, conflict of interest, etc...) apply to your program, as the person running it is incentivized to keep it running and keep a copy of the program. It's cheaper for the user running it to not have security they don't need. You don't need a bank-grade vault to store a $1 bill!

</pet peeve>

Pandapip1
  • 730
  • 5
  • 15
  • Thanks, the solution you linked is the same I have already implemented. It works, it's just not efficient. I like the concept of Lazy Evaluation as the solution, however mapping requires storage and can't be done in memory. – mattblack Jun 01 '22 at 22:32