ditto-lab / ditto

NFT Future protocol
MIT License
60 stars 3 forks source link

Hardcoded gas limit can break on future gas cost change #50

Open 0xbok opened 2 years ago

0xbok commented 2 years ago

https://github.com/ditto-lab/ditto/blob/2dd8b7f20a0f223160466a8d2953551a796c5306/src/DittoMachine.sol#L592

calvbore commented 1 year ago

I'm wondering if we should increase the gas stipend? or if there is some way we can allow a user to increase some minimum amount.

0xbok commented 1 year ago

how about taking it as a function argument? any new attack vectors here?

calvbore commented 1 year ago

yeah, if there's a protocol built with the token ejector and and 0 is passed as the arg then none of their business logic will be executed, so there needs to be a minimum guaranteed amount of gas fro execution.

0xbok commented 1 year ago

Allowing user to increase minimum gas amount through an argument will not work in cases where the nft seller is not benefitting from ejector code execution. They'll always prefer the minimum gas amount.

Thinking out loud: what if we let the clone owner send eth to DittoMachine to be used as gas when ejector is called. So there is a mapping(cloneId => value) and anyone can bump up the value. I can't decide if this is a good or a bad idea.