search

ethereum / solidity

Solidity, the Smart Contract Programming Language
https://soliditylang.org
GNU General Public License v3.0
23.17k stars 5.75k forks source link

Remove ABIEncoderV1 #11323

Open axic opened 3 years ago

axic commented 3 years ago

The eventual goal of #9609 was to phase out the old ABI coder. Creating this as a tracking issue.

IIRC in the discussions we aimed to remove the old one two breaking releases post making ABI coder V2 the default. That change took place in 0.8.0, which means we could drop ABI coder V1 in 0.10.0.

axic commented 3 years ago

It would be nice to know if anyone still depends on V1 and if so, what are the reasons for it?

axic commented 3 years ago

After a second look, this may not bring too many benefits, as there's just so many places in the code not worth touching. Removing the entirety of the old codegen is easier once the IR-based one is mature. However just disabling the V1 codepath may be useful to lessen the complexity of tests.

cameel commented 3 years ago

Isn't it too early to disable it before we retire the old codegen? I remember that we had some users that were intentionally avoiding v2 and relying on v1 for gas savings. See for example this conversation: https://gitter.im/ethereum/solidity?at=6039240be562cf54ac85b541

EDIT: Or maybe it's OK actually. Reading further, it turned out that results were inconsistent. Sometimes v1 was cheaper, sometimes v2: https://gitter.im/ethereum/solidity?at=6039389995e23446e4fbdc56

axic commented 3 years ago

@cameel that discussion seem to have concluded that the optimiser was not turned (due to a typo) and v2 was actually cheaper than v1 with the optimiser on :wink:

cameel commented 3 years ago

Yeah. Looks I misremembered that situation. I mostly remembered that we had someone who specifically wanted v1.

Still, the thread is probably a good piece of info either way. If v2 is actually cheaper in real-life situations that's even better and a good argument for disabling v1.

axic commented 3 years ago

I would propose that we could start issuing a warning for when someone uses an imported source which defines pragma abicoder v1; explicitly, while the outside is v2 (by default or explicitly).

We should have made this a warning with 0.8.0. It is not something which existed prior, as v1 was implicit and no explicit marking existed for it. I wonder if there is any legitimate use case where this scenario is required and is not done by mistake?