Skip to content

Conversation

@febo
Copy link
Contributor

@febo febo commented Sep 12, 2025

Problem

In some cases, comparing addresses ([u8;32] arrays) with the equality operator (==) results in "bloated" code, which eventually leads to an increased CU consumption.

Solution

This PR adds a custom PartialEq implementation to Address, which compares its value as a sequence of u64 values.

Copy link
Collaborator

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks good!

There was a comment in p-token about not using it in one particular place: https://github.com/solana-program/token/pull/90/files#diff-d65a3839b162d49521160ef473d81b3965eedffa48c0b7434ee324993951805aR13

Is the idea that this is better in almost all situations, but in rare cases it will be faster to use the auto-derived implementation?

@febo
Copy link
Contributor Author

febo commented Sep 13, 2025

Looks good!

There was a comment in p-token about not using it in one particular place: https://github.com/solana-program/token/pull/90/files#diff-d65a3839b162d49521160ef473d81b3965eedffa48c0b7434ee324993951805aR13

Is the idea that this is better in almost all situations, but in rare cases it will be faster to use the auto-derived implementation?

Probably, I only found that one place that was worse in terms of CU. I think it is difficult to tell since it all depends on what optimizations the compiler is doing. We will be able to "access" the default one by doing p1.as_array() == p2.as_array().

Lucas is having a look at what the compiler is doing wrong on the default ==, so it might be the case that we won't need the custom implementation in the future.

Copy link
Contributor

@rustopian rustopian left a comment

Choose a reason for hiding this comment

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

Looks good here.

EDIT: #345

@febo febo merged commit abd58a0 into anza-xyz:master Sep 16, 2025
25 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.

4 participants