vyperlang / vyper

Pythonic Smart Contract Language for the EVM
https://vyperlang.org
Other
4.81k stars 788 forks source link

feat[lang]: rename `_abi_encode` and `_abi_decode` #4097

Closed charles-cooper closed 3 weeks ago

charles-cooper commented 4 weeks ago

What I did

How I did it

How to verify it

Commit message

rename to `abi_encode` and `abi_decode` respectively
leave the old ones in, but with deprecation warnings

Description for the changelog

Cute Animal Picture

![Put a link to a cute animal picture inside the parenthesis-->]()

z80dev commented 4 weeks ago

🔥

cyberthirst commented 4 weeks ago

are we abandoning the idea of using abi.encode / abi.decode?

cyberthirst commented 4 weeks ago

i think we can update the docs too in this PR

cyberthirst commented 4 weeks ago

would like to see at least a few tests for the new variant

cyberthirst commented 4 weeks ago

do we need to display this to the users?

vyper_warn(f"`{self._id}()` is deprecated! Please use `{super()._id}()` instead.", node)

from:

UserWarning: `_abi_decode()` is deprecated! Please use `abi_decode()` instead.

  contract "tests/custom/test4.vy:4", function "run", line 4:52 
       3     y: Bytes[2 * 32 + 3 * 32 + 3 * 32 * 4] = x
  ---> 4     decoded_y1: DynArray[DynArray[uint256, 3], 3] = _abi_decode(
  -----------------------------------------------------------^
       5         y,

  vyper_warn(f"`{self._id}()` is deprecated! Please use `{super()._id}()` instead.", node)
cyberthirst commented 4 weeks ago

nit: https://github.com/vyperlang/vyper/blob/7ccd951aca1b8ea98d2e699ca23bd95055d0280e/vyper/builtins/functions.py#L2338-L2339

cyberthirst commented 4 weeks ago

this won't be problematic? https://github.com/vyperlang/vyper/blob/24cfe0bcc10ec9418054c8def5cf4eeaa3ed0164/vyper/ast/grammar.lark#L300

fubuloubu commented 3 weeks ago

are we abandoning the idea of using abi.encode / abi.decode?

would love to have abi module namespaced under ethereum stdlib module

charles-cooper commented 3 weeks ago

do we need to display this to the users?

vyper_warn(f"`{self._id}()` is deprecated! Please use `{super()._id}()` instead.", node)

i think that's just a function of how warnings.warn() works, we can investigate how to squash them but it's unrelated to this PR

charles-cooper commented 3 weeks ago

this won't be problematic?

https://github.com/vyperlang/vyper/blob/24cfe0bcc10ec9418054c8def5cf4eeaa3ed0164/vyper/ast/grammar.lark#L300

hmm yea, it will make some tests fail once we update the usages in the test suite

charles-cooper commented 3 weeks ago

are we abandoning the idea of using abi.encode / abi.decode?

yea, at least for now. it's a less trivial change, we have no machinery for it right now