SuperblocksHQ / ethereum-studio

Official Ethereum Studio project repository. And IDE specially tailored to make it as simple as possible to onboard new users into the Ethereum ecosystem
https://ethereum.org/build/
GNU General Public License v3.0
185 stars 102 forks source link

Change CyptoPizza.sol isUnique modifier to O(1) complexity #273

Open ernestognw opened 3 years ago

ernestognw commented 3 years ago

Summary

Crypto Pizza example project has a isUnique function that traverses the whole array of Pizzas registered. This is a O(n) behaviour that could be simplified by creating a nameToPizzaId and dnaToPizzaId mapping to achieve O(1) complexity.

Motivation

Since this is an example project for people starting to use EthereumStudio, I don't think it'll be nice to encourage this sort of searches in a modifier function. Also, I was digging in the Solidity docs to see if modifiers somehow don't consume gas, but as far as explained in this answer, they do.

Describe alternatives you've considered

We can replace the isUnique modifier content with a O(1) check to nameToPizzaId and dnaToPizzaId mappings, in order to keep function working the same but with significantly less gas consumption.

I can implement it if there's no specific reason to keep it as it is.

Additional context

The isUnique function can be seen here

And here's the solidity documentation about modifiers. This is where I couldn't find any reason why it could be accepted to have an O(n) behaviour

shreyas-londhe commented 2 years ago

I want to work on this