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

fix encodePacked DynamicBytes #2042

Merged
merged 13 commits into from
Apr 25, 2024
Merged

fix encodePacked DynamicBytes #2042

merged 13 commits into from
Apr 25, 2024

Conversation

xutruth
Copy link
Contributor

@xutruth xutruth commented Apr 18, 2024

What does this PR do?

fix encodePacked DynamicBytes

In solidity, abi.encodePacked function is used to pack the given parameters into a tightly packed byte array without inserting length information. This function is commonly used for creating hashes or signatures of data in Solidity contracts, It does not align the data to 32 bytes like abi.encode.

//  ethers   ^6.10.0
import {AbiCoder, ethers} from "ethers";
let packed = ethers.solidityPacked(["bytes", "bytes"], ["0x12", "0x12"])
console.log(packed)  //0x1212

Where should the reviewer start?

  1. TypeEncoder#encodePacked
  2. DefaultFunctionEncoderTest#testEncodeConstructorPacked_multipleParameters
  3. TypeEncoderPackedTest#testDynamicBytesEncodePacked

Why is it needed?

fix encodePacked DynamicBytes -> solidity bytes

Checklist

  • I've read the contribution guidelines.
  • I've added tests (if applicable).
  • I've added a changelog entry if necessary.

gtebrean and others added 10 commits April 18, 2024 22:04
Signed-off-by: gtebrean <99179176+gtebrean@users.noreply.github.com>
Signed-off-by: gtebrean <99179176+gtebrean@users.noreply.github.com>
Signed-off-by: gtebrean <99179176+gtebrean@users.noreply.github.com>
Signed-off-by: gtebrean <99179176+gtebrean@users.noreply.github.com>
Signed-off-by: gtebrean <99179176+gtebrean@users.noreply.github.com>
Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Signed-off-by: Nischal Sharma <nischal@web3labs.com>
Signed-off-by: hamburger <1210062377@qq.com>
@NickSneo
Copy link
Contributor

@xutruth Thanks for contributing. Can you plz add a changelog entry for this?

Signed-off-by: hamburger <1210062377@qq.com>
@xutruth
Copy link
Contributor Author

xutruth commented Apr 22, 2024

@xutruth Thanks for contributing. Can you plz add a changelog entry for this?

@NickSneo I have added chnagelog for this commit!

@NickSneo
Copy link
Contributor

@xutruth Hey,

Can you add link to the references for examples or tests you added of encodePacked DynamicBytes.
I found these - https://docs.ethers.org/v6/api/hashing/#solidityPacked
https://docs.soliditylang.org/en/v0.8.14/abi-spec.html#non-standard-packed-mode

Signed-off-by: hamburger <1210062377@qq.com>
@xutruth
Copy link
Contributor Author

xutruth commented Apr 23, 2024

I also refer to the official solidity documentation,v0.8.25, There are two sentences on how to encode strings and bytes

  • Dynamically-sized types like string, bytes or uint[] are encoded without their length field.
  • The encoding of string or bytes does not apply padding at the end, unless it is part of an array or struct (then it is padded to a multiple of 32 bytes).

but if use abi.encodePacked(struct) will be Error (9578): Type not supported in packed mode.

So I modified the handling of Utf8String and DynamicBytes,plz refer to this submission b1ad81b6 ,I added a comment “removePadding can also be used, but is not necessary“

@xutruth
Copy link
Contributor Author

xutruth commented Apr 23, 2024

I also refer to the official solidity documentation,v0.8.25, There are two sentences on how to encode strings and bytes

  • Dynamically-sized types like string, bytes or uint[] are encoded without their length field.
  • The encoding of string or bytes does not apply padding at the end, unless it is part of an array or struct (then it is padded to a multiple of 32 bytes).

but if use abi.encodePacked(struct) will be Error (9578): Type not supported in packed mode.

So I modified the handling of Utf8String and DynamicBytes,plz refer to this submission b1ad81b6 ,I added a comment “removePadding can also be used, but is not necessary“

@NickSneo

@xutruth
Copy link
Contributor Author

xutruth commented Apr 23, 2024

I also refer to the official solidity documentation,v0.8.25, There are two sentences on how to encode strings and bytes

  • Dynamically-sized types like string, bytes or uint[] are encoded without their length field.
  • The encoding of string or bytes does not apply padding at the end, unless it is part of an array or struct (then it is padded to a multiple of 32 bytes).

but if use abi.encodePacked(struct) will be Error (9578): Type not supported in packed mode.
So I modified the handling of Utf8String and DynamicBytes,plz refer to this submission b1ad81b6 ,I added a comment “removePadding can also be used, but is not necessary“

@NickSneo
In fact, I tried it in solidity, ethers.js, and web3.js

Copy link
Contributor

@NickSneo NickSneo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gtebrean gtebrean merged commit 6847173 into hyperledger:main Apr 25, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants