Skip to content

Add support to visually represent addresses and hashes with 'blo' #44

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

Open
wants to merge 53 commits into
base: main
Choose a base branch
from

Conversation

Archethect
Copy link

As requested in #14, this PR leverages the same library for representing EVM addresses as used in the Safe UI.

I also added support to visually represent the hashes in the same way. Feel free to tell me if you'd like to have this removed if you don't like it @josepchetrit12, @xaler5

Screenshot 2025-03-26 at 11 16 53 Screenshot 2025-03-26 at 11 17 36 Screenshot 2025-03-26 at 11 17 46

Copy link

github-actions bot commented Mar 26, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@Archethect
Copy link
Author

I confirm that I have read and hereby agree to the OpenZeppelin Contributor License Agreement

@josepchetrit12
Copy link
Collaborator

Great! Awesome work! Since Safe doesn't use this pixel for hashes, I'd prefer to remove it. Besides, in theory, this library was created for addresses.

I thought maybe there could be a collision, but I tested it and it didn't:

const address = "0x111CEEee040739fD91D29C34C33E6B3E112F2177"
const hash32 = "0x000000000000000000111CEEee040739fD91D29C34C33E6B3E112F2177"

I think I'd also reduce the size of the pixel
image

@josepchetrit12
Copy link
Collaborator

Maybe we also want to round the corners like in safe, what do you think?
image

@Archethect
Copy link
Author

Great! Awesome work! Since Safe doesn't use this pixel for hashes, I'd prefer to remove it. Besides, in theory, this library was created for addresses.

I thought maybe there could be a collision, but I tested it and it didn't:

const address = "0x111CEEee040739fD91D29C34C33E6B3E112F2177"
const hash32 = "0x000000000000000000111CEEee040739fD91D29C34C33E6B3E112F2177"

Gotcha. I also checked this, and it just expects any string starting with '0x', therefore both hashes and addresses work.
I'll remove it for hashes.

Maybe we also want to round the corners like in safe, what do you think?
image

Yes, certainly doable! Regarding the size, let's see what is still visible.

@Archethect
Copy link
Author

ok @josepchetrit12 ,

Tell me what you like:

30 px:
Screenshot 2025-03-26 at 13 02 45

25 px:
Screenshot 2025-03-26 at 13 03 15

20 px:
Screenshot 2025-03-26 at 13 02 28

@josepchetrit12
Copy link
Collaborator

20px it's the way to go, network image and pixel avatar should be the same size and style!

@Archethect
Copy link
Author

ok cool! Change is committed.

josepchetrit12
josepchetrit12 previously approved these changes Mar 27, 2025
Copy link
Collaborator

@josepchetrit12 josepchetrit12 left a comment

Choose a reason for hiding this comment

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

LGTM! Amazing thanks for your contribution!

@josepchetrit12 josepchetrit12 requested a review from xaler5 March 27, 2025 08:27
Copy link

@ferrabled ferrabled left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the changes. Just one small note: there are conflicts in package-lock.json with the main branch (a dependency that it is not being used). We should regenerate the lock file to clean up any unused dependencies to maintain the dependency tree clean and consistent.

Signed-off-by: Fernando Rabasco Ledesma <[email protected]>
ferrabled
ferrabled previously approved these changes May 6, 2025
josepchetrit12
josepchetrit12 previously approved these changes May 6, 2025
Copy link
Collaborator

@josepchetrit12 josepchetrit12 left a comment

Choose a reason for hiding this comment

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

LGTM!

@josepchetrit12
Copy link
Collaborator

@Archethect Can you sign your commits? so we can merge this PR! thanks

@Archethect
Copy link
Author

@Archethect Can you sign your commits? so we can merge this PR! thanks

Verified/Signed my last noop commit.
Let me know if this is good to go, or you need me to sign all my previous commits as well.
I think if I do that, all other commits will need new signatures because of the rebase though.

@josepchetrit12
Copy link
Collaborator

josepchetrit12 commented May 7, 2025

@Archethect Can you sign your commits? so we can merge this PR! thanks

Verified/Signed my last noop commit. Let me know if this is good to go, or you need me to sign all my previous commits as well. I think if I do that, all other commits will need new signatures because of the rebase though.

We need all the commits signed. @Archethect

josepchetrit12 and others added 25 commits May 7, 2025 14:55
* take the more relevant method

* convert value to string
* add subtitle

* test signature

* re-work how-it-works

* remove disclaimer

* restore mb
* add thumbnail

* add extension
* move image to public folder

* remove extra content
* add request audit button

* format page
Bumps [axios](https://github.com/axios/axios) from 1.8.1 to 1.8.2.
- [Release notes](https://github.com/axios/axios/releases)
- [Changelog](https://github.com/axios/axios/blob/v1.x/CHANGELOG.md)
- [Commits](axios/axios@v1.8.1...v1.8.2)

---
updated-dependencies:
- dependency-name: axios
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* fetch safe version in api

* add old domain separator
Bumps [next](https://github.com/vercel/next.js) from 14.2.25 to 14.2.26.
- [Release notes](https://github.com/vercel/next.js/releases)
- [Changelog](https://github.com/vercel/next.js/blob/canary/release.js)
- [Commits](vercel/next.js@v14.2.25...v14.2.26)

---
updated-dependencies:
- dependency-name: next
  dependency-version: 14.2.26
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@Archethect Archethect dismissed stale reviews from josepchetrit12 and ferrabled via 8e0af32 May 7, 2025 13:07
@Archethect
Copy link
Author

@Archethect Can you sign your commits? so we can merge this PR! thanks

Verified/Signed my last noop commit. Let me know if this is good to go, or you need me to sign all my previous commits as well. I think if I do that, all other commits will need new signatures because of the rebase though.

We need all the commits signed. @Archethect

@ferrabled @josepchetrit12 I rebased and signed all my commits. Let me know if anything else is required. 🙏

Copy link
Collaborator

@josepchetrit12 josepchetrit12 left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

8 participants