gnosis / conditional-tokens-contracts

Smart contracts for conditional tokens.
GNU Lesser General Public License v3.0
163 stars 64 forks source link

add public ID calculation helpers #47

Closed cag closed 5 years ago

cag commented 5 years ago

@xavierlepretre

So it looks like deduplicating the IDs and exposing them as part of the public interface would indeed decrease code size, but it would also increase the cost per transaction for everything (as there would be a JMP to and from the deduplicated code section instead of having the logic being "inlined").

Also, since these calls are public, there would be additional ABI parsing logic prepended to those code sections and JMPDESTS for their corresponding function selectors, so I was curious if that overhead would cancel the deduplication size, but as you can see, deployment costs were reduced, so it looks like overall, the deduplication had the intended effect on code size.

Closes #31

xavierlepretre commented 5 years ago

Your comments make me think that 2 good things that could be added to Solidity are:

xavierlepretre commented 5 years ago

I am surprised by the (large) amount of gas being added by way of JMP.

cag commented 5 years ago

Yeah, well, it is two JMPs, so that is 16 gas, but then there is also preparing the stack. (I wonder if the declaration order will affect things...) There's also saving and loading the JUMPDEST of the return location for each call, which I think is done to memory, and since the optimizer is off, I assume there are many redundant security features which are meant to be optimized away according to some rules or something like that. This other stuff comes out to about 60 gas without the optimizer...

I wonder how the optimizer would affect this.

cag commented 5 years ago

Crazy: there is a 13 gas difference in splitPosition after the optimizer has been enabled. That means that must have differed in one JMP at max, though I don't know how that happened. Anyway, the optimizer saved enough gas that I think this addition is justifiable.