fix: align address decoding with Solidity canonical behavior #803
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wjmelements
reviewed
Nov 14, 2025
wjmelements
reviewed
Nov 14, 2025
pdp/contract/utils_test.go
Outdated
| t.Errorf("Case 2 failed: Expected %x, got %x", input2, decoded2.Bytes()) | ||
| } | ||
|
|
||
| // Case 3: Input longer than 20 bytes → use last 20 bytes only |
Collaborator
There was a problem hiding this comment.
don't enumerate comments
wjmelements
reviewed
Nov 14, 2025
pdp/contract/utils.go
Outdated
| func DecodeAddressCanonical(input []byte) common.Address { | ||
| b := make([]byte, 20) | ||
| inLen := len(input) | ||
| if inLen >= 20 { |
Author
|
@wjmelements done with the changes |
wjmelements
reviewed
Nov 14, 2025
pdp/contract/utils.go
Outdated
| // DecodeAddressCapability implements the Solidity behavior of | ||
| // address(uint160(BigEndian.decode(input))). | ||
| func DecodeAddressCapability(input []byte) common.Address { | ||
| // If input is longer, we must take the *head* of the slice first. |
wjmelements
reviewed
Nov 14, 2025
pdp/contract/utils_test.go
Outdated
| t.Errorf("Case 2 (exact) failed: Expected %s, got %s", expected2.Hex(), decoded2.Hex()) | ||
| } | ||
|
|
||
| // Case 3: Input > 20 bytes but <= 32 bytes → use last 20 bytes |
wjmelements
reviewed
Nov 14, 2025
pdp/contract/utils_test.go
Outdated
| t.Errorf("Case 3 (medium) failed: Expected %s, got %s", expected2.Hex(), decoded3.Hex()) | ||
| } | ||
|
|
||
| // Case 4: Input > 32 bytes → take first 32, then last 20 from that |
wjmelements
reviewed
Nov 14, 2025
pdp/contract/utils_test.go
Outdated
| t.Errorf("Case 1 (short) failed: Expected %s, got %s", expected1.Hex(), decoded1.Hex()) | ||
| } | ||
|
|
||
| // Case 2: Exactly 20 bytes input → use as is |
wjmelements
reviewed
Nov 14, 2025
pdp/contract/utils_test.go
Outdated
|
|
||
| // TestDecodeAddressCapability tests the canonical address decoding. | ||
| func TestDecodeAddressCapability(t *testing.T) { | ||
| // Case 1: Input shorter than 20 bytes → left pad with zeros |
wjmelements
reviewed
Nov 14, 2025
pdp/contract/utils_test.go
Outdated
| // These 8 bytes should be IGNORED (they are after byte 32) | ||
| 0xEE, 0xEE, 0xEE, 0xEE, 0xEE, 0xEE, 0xEE, 0xEE, | ||
| } | ||
| // The expected result is the last 20 bytes *of the first 32 bytes* |
Collaborator
There was a problem hiding this comment.
Please consult the test cases I provided for you
#758 (comment)
wjmelements
reviewed
Nov 14, 2025
pdp/contract/utils_test.go
Outdated
| t.Errorf("Case 1 (short) failed: Expected %s, got %s", expected1.Hex(), decoded1.Hex()) | ||
| } | ||
|
|
||
| // Case 2: Exactly 20 bytes input → use as is |
Collaborator
There was a problem hiding this comment.
just make these into separate test cases instead of cramming them into the same test case
Author
|
@wjmelements done with the changes |
wjmelements
requested changes
Nov 15, 2025
Collaborator
wjmelements
left a comment
wjmelements
left a comment
There was a problem hiding this comment.
Kindly read the instructions more carefully. If you have been using AI, please stop.
pdp/contract/utils_test.go
Outdated
| 0xEE, 0xEE, 0xEE, 0xEE, 0xEE, 0xEE, 0xEE, 0xEE, | ||
| } | ||
|
|
||
| // The expected result is the last 20 bytes *of the first 32 bytes*, which is expectedAddr |
Author
|
@wjmelements I’ve pushed the updated changes now. Just to clarify — I did not use any AI for this. I was a bit confused earlier, so I took some extra time to understand the instructions properly. Everything in this update is written and reviewed by me. Please let me know if any further adjustments are needed. |
Author
|
@wjmelements done with the changes |
wjmelements
approved these changes
Nov 16, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes inconsistent decoding of Ethereum addresses from byte slices in the ServiceProviderRegistry and related code, addressing issue #758.
DecodeAddressCanonicalfunction matching Solidity'saddress(uint160(BigEndian.decode(bytes))).Issue
Fixes #758
Please review the decoding logic and scope of changes. Happy to include tests or make adjustments as needed.