Skip to content

Gas Optimizations #15

@mudgen

Description

@mudgen

Various gas optimizations without resorting to assembly can be made.
Basic information about gas optimization is in this article: https://eip2535diamonds.substack.com/p/smart-contract-gas-optimization-with
The data from that article could be applied to all code.

For gas optimization I only reviewed the function below and give my observations:

function craftInstallations(uint256[] calldata _installationTypes) external {
address[4] memory alchemicaAddresses = RealmDiamond(s.realmDiamond).getAlchemicaAddresses();
for (uint i = 0; i < _installationTypes.length; i++) {
require(_installationTypes[i] < s.installationTypes.length, "InstallationFacet: Installation does not exist");
InstallationType memory installationType = s.installationTypes[_installationTypes[i]];
//level check
require(installationType.level == 1, "InstallationFacet: can only craft level 1");
require(!installationType.deprecated, "InstallationFacet: Installation has been deprecated");
//take the required alchemica
for (uint j = 0; j < installationType.alchemicaCost.length; j++) {
LibERC20.transferFrom(
alchemicaAddresses[j],
msg.sender,
s.realmDiamond,
installationType.alchemicaCost[j]
);
}
if (installationType.craftTime == 0) {
LibERC1155._safeMint(msg.sender, _installationTypes[i], 0);
} else {
uint256 readyBlock = block.number + installationType.craftTime;
//put the installation into a queue
//each wearable needs a unique queue id
s.craftQueue.push(QueueItem(s.nextCraftId, readyBlock, _installationTypes[i], false, msg.sender));
emit AddedToQueue(s.nextCraftId, _installationTypes[i], readyBlock, msg.sender);
s.nextCraftId++;
}
}
//after queue is over, user can claim installation
}

Line 216 repeatedly reads a state variable when it doesn't need to. Instead s.installationTypes.length could be assigned to a local variable outside the loop and the local variable could be used in the loop.

Line 218 InstallationType memory installationType = s.installationTypes[_installationTypes[i]]; is very gas inefficient because it reads all the state variables from the InstallationType struct from contract storage but not all the variables are used in the function, and it does this within a loop. I know I said to assign to a memory variable like this. I was wrong.

Line 242 is updating a state variable in a loop. It could be updated one time after the loop. Before the loop a local variable could be assigned the value of s.nextCraftId and be used and updated within the loop. The local variable could also be read at line 239 instead of the s.nextCraftId state variable.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions