Too much gas consumption

Question

I'm programming my first Eth Contract, I'm facing a problem, the gas consumption ( estimated ) of the buy method is going really to high ( Quickly > N million gas before getting the "Maximum gas allowance exceeded" error. )

To be quick, the idea is the following:

• There is a map ( 2D map ) with multiple zones that you can own ( called units here, this is why I maintain a "unitsToState" obvious mapping ).

• You can purchase multiple adjacent zones at once, so a "Block" is created.

• So when you purchase a new Block the contract must check if all the units inside are empty ( unitsToState[x] == 0 ). When the block is purchased, these states are set to 1.

I don't explain too much details here, because I guess the problem is mainly "Solidity" bad algorithm programming from me.

This method can be executed with arround 500k gas for fromX, fromY, toX, toY that represent a small zone, but when those are far from each other, I got the "Maximum gas allowance exceeded" error during my gas estimation .. So really I guess there is a problem ..

``` ...

struct Block {
    address owner;
    uint fromX;
    uint fromY;
    uint toX;
    uint toY;
    string imageUrl;
    string redirectUrl;
    string text;
    bool removed;
}   

uint size = 100;
mapping (uint => uint) unitsToState;
Block[] public blocks;
uint public areaPrice;
uint public areaPerUnit;

...


function buy(uint fromX, uint fromY, uint toX, uint toY, string imageUrl, string redirectUrl, string text) payable public {
    require(fromX >= 0);
    require(fromY >= 0);
    require(fromX <= toX);
    require(fromY <= toY);
    require(toX < size);
    require(toY < size);

    // Here do check of collisions.
    for (uint i = fromX; i <= toX; i++) {
        for (uint j = fromY; j <= toY; j++) {
            require(getUnitsToState(i*size*size + j) == 0);
        }    
    }

    uint width = toX - fromX + 1;
    uint height = toY - fromY + 1;
    uint areaCount = width * height * areaPerUnit;
    uint price = areaCount * areaPrice;
    require(msg.value >= price);

    Block memory b = Block(
       msg.sender,
       fromX,
       fromY,
       toX,
       toY,
       imageUrl,
       redirectUrl,
       text,
       false
    );
    blocks.push(b);

    // Registrer units states.
    for (i = fromX; i <= toX; i++) {
        for (j = fromY; j <= toY; j++) {
            unitsToState[i*size*size + j] = 1;
        }    
    }
}

...

EDIT: The problem seems to be in greatest part from the part after "// Registrer units states.". But ... it doesn't really help me as long as I needs to do that ..

Answer 1

I suspect this is simply the cost of doing what you're doing. IIRC, the gas cost for changing a zero value to a non-zero is 20,000. A 10x10 block would require storing 10*10=100 ones, and 100*20000 is 2,000,000 gas.

Assuming that's roughly consistent with what you're seeing, that's simply what it costs to store this data.

Answer 2

Your iterative processes are increasing the cost of interactions relative to scale. This won't do. It's too expensive and will ultimately fail. More on why this is anti-pattern: https://blog.b9lab.com/getting-loopy-with-solidity-1d51794622ad

This part

// Here do check of collisions.
for (uint i = fromX; i <= toX; i++) {
    for (uint j = fromY; j <= toY; j++) {
        require(getUnitsToState(i*size*size + j) == 0);
    }    
}

can be refactored to complete in a single step (at any scale) by setting up a data structure to support a one-step lookup.

// x => y => value
mapping(uint => mapping(uint => uint)) public xy;

Set the non-zero values as you go.

This part can be engineered out of the design.

// Registrer units states.
for (i = fromX; i <= toX; i++) {
    for (j = fromY; j <= toY; j++) {
        unitsToState[i*size*size + j] = 1;
    }    
}

This is setting everything to a default value of 1. You automatically get a default value of 0 at no cost, so use that and create your own getter functions to implement the offset.

function setThing(uint value) public {
  thing = value - 1;
}

function getThing() public returns(uint) {
  return thing + 1;
}

Hope it helps.