Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

abi-coder support for fixed, ufixed and function types? #389

Open
theethernaut opened this issue Jan 6, 2019 · 7 comments
Open

abi-coder support for fixed, ufixed and function types? #389

theethernaut opened this issue Jan 6, 2019 · 7 comments
Labels
enhancement New feature or improvement.

Comments

@theethernaut
Copy link

As far as I can tell, the abi-coder util supports all types listed in Solidity's documentation: https://solidity.readthedocs.io/en/v0.4.24/abi-spec.html#types

Except fixed, ufixed and function types, that is. Can you confirm this? And if so, are you planning to support these types?

@ricmoo
Copy link
Member

ricmoo commented Jan 6, 2019

That’s actually the oldest issue I opened 2 years ago, #3

:)

Is fixed still supported by Solidity? There was discussion at one point about removing it.

There are a lot of complications and potentially non-deterministic outputs when supporting fixed-point operations, depending on the optimizations used. Another issue (and this impacts non-fixed as well, so maybe moot) is the difference in computation for compile-time vs run-time evaluation...

If added, it may be a fairly opaque type, that would not intrinsically support mathematical operations, but could be cast to support them.

What type of support do you require/suggest?

@ricmoo ricmoo added the discussion Questions, feedback and general information. label Jan 6, 2019
@ricmoo
Copy link
Member

ricmoo commented Jan 6, 2019

(FYI: just confirmed fixed is still supported in 0.5)

@ricmoo
Copy link
Member

ricmoo commented Jan 6, 2019

Just found this little nugget to keep in mind. :)

"Fixed point numbers are not fully supported by Solidity yet. They can be declared, but cannot be assigned to or from."

https://solidity.readthedocs.io/en/v0.5.2/types.html?highlight=fixed#fixed-point-numbers

@theethernaut
Copy link
Author

theethernaut commented Jan 6, 2019

Thanks @ricmoo, I guess that 'non-full' support by Solidity is reason enough not to support it by the abi-coder. But what about function types?

What type of support do you require/suggest?

This question just arose while we were writing tests for ZeppelinOS's encodeCall utillity, which may use the abi-coder in the near future, and realized that the coder did not accept the mentioned types. The zos command line program accepts initializer function calls, and we want to make sure that our ABI encoding fully supports all the types that Solidity provides: https://github.com/zeppelinos/zos/blob/b6c93f11667c5bcc505506e4f21b0d35be54cfab/packages/lib/test/src/helpers/encodeCall.test.js

@ricmoo
Copy link
Member

ricmoo commented Jan 6, 2019

It does make sense to add... The object returned for now may be a bit bothersome to use, but I'll add support this week. :)

@ricmoo ricmoo added the enhancement New feature or improvement. label Jan 11, 2019
@ricmoo ricmoo removed the discussion Questions, feedback and general information. label Feb 12, 2019
@ricmoo
Copy link
Member

ricmoo commented Mar 7, 2019

@ajsantander Have you actually been able to use the fixed/ufixed types? I've added support, but cannot test it because Solidity fails with UnimplementedFeatureError: Fixed point types not implemented.

@cgewecke
Copy link

Hi @ricmoo

Were you ever able to look at decoding Function types?

There is a corresponding issue at web3 2826 about decoding them when they are event params.

Have opened a tentative PR to fix there by re-mapping Function types to bytes24 per the Solidity spec:

If external function types are used outside of the context of Solidity, they are treated as the function type, which encodes the address followed by the function identifier together in a single bytes24 type

... but am wondering if you've already looked at this topic and there's more to it (or you're already fixing here).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement.
Projects
None yet
Development

No branches or pull requests

3 participants