Add abi.encodeError
Fixes #14287
TODO:
- [ ] code deducpliation between
typeCheckABIEncodeCallFunctionandtypeCheckABIEncodeErrorFunction?- I'm not sure if there is a clean way of doing that. Considering there is already a lot of code duplication in the codebase, maybe that is ok.
- [x] tests
- [ ] review
Thank you for your contribution to the Solidity compiler! A team member will follow up shortly.
If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother.
If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix.
This CI error: https://app.circleci.com/pipelines/github/ethereum/solidity/33577/workflows/8c3156a6-35f8-4aeb-b6e3-18f80f4cb6cf/jobs/1512784 is because the test file name is wrong, see: test/libsolidity/syntaxTests/specialFunctions/encodeCall_fail_args_internal_function_pointer_for_uint copy.sol. There is a _copy in the filename of this test https://github.com/ethereum/solidity/pull/14974/files#diff-41100eaa90ef551806d2098a1c4f80fe0f86d9587edbfb6f1161c6d543f1f92dR1.
Needs some semantic tests as well. A changelog entry as well.
Yes, i'm still working on the testing part.
Edit: should be good now
Apparently, in the semantic tests, the returned bytes memory are processed in chunks of 32 bytes. So when the buffer has length ≡ 4 mod 0x20, we need to pad the expected output with zeros (28 bytes worth of zero exactly)
Is that supposed to happen?
Could be just another instance of #13989. The issue is about fixed bytes, but maybe it affects bytes too.
I think that soltest is just not padding byte arrays correctly.
@matheusaaguiar
- I added some references in the files you mentionned
- I fixed the merge conflicts and fixed what needed to be fixed
- I also merged previous functions into
typeCheckABIEncodeCallFunctionOrError(added the type object and some switches)
Pending the discussion in https://github.com/ethereum/solidity/issues/14287#issuecomment-2096501517, we decided to close this PR for now. But feel free to jump in there (or join one of our calls) and make the case for this approach and we can potentially revisit and revive this PR! But as a general policy, we should discuss and confirm the approach before implementations, to be sure we're on the same page about it.
@ekpyron This change to ERC-7751, if it was to happen (note, I'm a co-author of the ERC, so not unlikelly), would create a significant usecase of abi.encodeError.
Depending on the feedback this gets, I'd like to reconsider this feature.