-
Notifications
You must be signed in to change notification settings - Fork 458
chore: ABIEncoder - docs, tests, cleanup #707
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
chore: ABIEncoder - docs, tests, cleanup #707
Conversation
/// Almost identical to use of `web3.eth.abi.encodeParameter` in web3.js. | ||
/// Calling `web3.eth.abi.encodeParameter('string','test')` in web3.js will return the following: | ||
/// ``` | ||
/// 0x0000000000000000000000000000000000000000000000000000000000000020 | ||
/// 0000000000000000000000000000000000000000000000000000000000000004 | ||
/// 7465737400000000000000000000000000000000000000000000000000000000 | ||
/// ``` | ||
/// but calling `ABIEncoder.encodeSingleType(type: .string, value: "test" as AnyObject)` will return: | ||
/// ``` | ||
/// 0x0000000000000000000000000000000000000000000000000000000000000004 | ||
/// 7465737400000000000000000000000000000000000000000000000000000000 | ||
/// ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is basically what triggered me to write docs and make sure people will know how each function works.
Don't recall exactly what I was implementing but I expected that encodeSingleType
would encode dynamic types with data offset in front of the data itself but it didn't happen. Was wondering why? Is it a bug? Appears that it's just different from web3.js.
The 0x0000000000000000000000000000000000000000000000000000000000000020
in the first code block is the data offset. See tests for more examples.
While writing the doc I also came up with another idea that will allow us to get the ABI with data offset from this function. The idea is the following: add a new input argument dynamicDataOffset
that will be true
by default
encodeSingleType(type: ABI.Element.ParameterType, value: AnyObject, dynamicDataOffset: Bool = true)
Uses of this function across the library will have to be updated to with encodeSingleType(..., dynamicDataOffset: false)
.
On the other hand, this update is a breaking change that will be silently pushed and could potentially break other projects. So as a win-win situation I think it would make sense to push that idea with the new argument to the v4.
Fix DocC comment layout.
Delete redundant empty line within DocC comment.
Looks like I've had broke the test file when merged a previous PR not in its turn, sorry. |
Should be fixed now. Let's wait for the CI. |
Summary of Changes
convertToBigInt
andconvertToBigUInt
, and tests to check the presence of data offsets required for dynamic bytes;Test Data or Screenshots
By submitting this pull request, you are confirming the following:
develop
branch.