transmissions11 / solmate

Modern, opinionated, and gas optimized building blocks for smart contract development.
GNU Affero General Public License v3.0
3.93k stars 645 forks source link

Update ERC1155.sol:⚡Optimized For loops #390

Closed 0xScratch closed 1 year ago

0xScratch commented 1 year ago

It be great not to initialize uint256 i to 0, cuz the default values of all unsigned integers usually stick to 0..just a good practice I guess.

Refer this -> https://gist.github.com/grGred/9bab8b9bad0cd42fc23d4e31e7347144#for-loops-improvement

These changes won't really bring out some bugs or errors. Thus, I didn't felt any need of running the tests which were mentioned!

transmissions11 commented 1 year ago

my understanding is this does not save gas, and is slightly unintuitive, so does not feel worth it to me

0xScratch commented 1 year ago

As your wish!!

0xScratch commented 1 year ago

Just a query for learning point of view:

Here are two code snippets from this solidity file:

// Unchecked because the only math done is incrementing
// the array index counter which cannot possibly overflow.
unchecked {
    for (uint256 i = 0; i < owners.length; ++i) {
        balances[i] = balanceOf[owners[i]][ids[i]];
    }
}

and

for (uint256 i = 0; i < ids.length; ) {
    id = ids[i];
    amount = amounts[i];

     balanceOf[from][id] -= amount;
     balanceOf[to][id] += amount;

     // An array can't have a total length
     // larger than the max uint256 value.
     unchecked {
         ++i;
     }
}

In the first code snippet, you dropped down the whole for loops inside that unchecked block, but in the second you just kept that incrementing factor ++i Like why it is done in this way, why apply the same method in both...Well, I know it's due to some overflow errors, but still want to know it by depth

@transmissions11 feel free to answer this, if you got time!

malik672 commented 1 year ago

@Aryan9592 because two can probably have underflows and overflows why one can't, i mean look at both loops, one of them performs an arithmetic operation in it, why the other does not

0xScratch commented 1 year ago

got it!