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

Display Checksummed Address for all addresses #6

Open
brianmcmichael opened this issue Apr 13, 2022 · 4 comments
Open

Display Checksummed Address for all addresses #6

brianmcmichael opened this issue Apr 13, 2022 · 4 comments
Assignees

Comments

@brianmcmichael
Copy link

When displaying addresses, we should use the checksummed address whenever displaying an address.

Note the following example:

Screenshot from 2022-04-13 08-11-56

where the checksummed address is 0xddD44166CdCd051AFa053A716608Cdc42C0C30D4

Compilation will fail if the non-checksummed address is used so this will save some dev steps for those importing addresses to their contracts.

@julienmartinlevrai
Copy link
Contributor

julienmartinlevrai commented Apr 20, 2022

Implementing this feature has tradeoffs we need to consider.

As checksummed addresses cannot be obtained on-chain, they need to be calculated off-chain.

In order to do that, we need to import a JavaScript library that provides an implementation of the keccak hashing algorithm.

Front-end library management is non-trivial because the library code needs to be bundled together with the rest of the code. This is usually done with a tool such as webpack that allows you to have multiple sources and then bundles them together in the final webpage that gets sent to the user as a front-end.

This will add many lines of code to the front-end, which will make it slower to load and harder to audit. A common improvement for front-end loading times is to minify the source code, but that will make it completely unreadable for humans.

As this is critical infrastructure, I thought it was a good tradeoff to leave checksummed addresses out. However, you are right it makes this front-end incompatible with Solidity, which requires checksummed addresses to work.

Right now, you can right-click on the webpage, "View page source", and get the Javascript code exactly as it appears in the Github repo. This improves the audit trail and lets you review the code directly in the browser. That ability would get lost because the code wouldn’t be the same and it would be harder to read.

But maybe in practice nobody really does this kind of auditing before using a front-end lol (but they should).

Edit: I just noticed you included a link to the Web3.js library documentation. Just to clarify, the Chainlog UI does not use that library or any other library for the above reasons. Instead, it makes HTTP requests to the EVM provider using JSON RPC calls.

@julienmartinlevrai
Copy link
Contributor

I just thought of a middle-ground solution. Create a centralized API that calculates the checksum and consume that from the UI. The UI will not be able to check the checksum correctness, but it will check that the address was not altered. Worst-case scenario if the API gets compromised, the checksum will be wrong but the address will still be correct. What do you think @brianmcmichael  ?

@julienmartinlevrai
Copy link
Contributor

@brianmcmichael done

2022-05-13-133108_691x113_scrot

some work remaining for bulk operations though

@cristidas
Copy link
Contributor

@brianmcmichael are you happy with the solution? Can this issue be closed as resolved now?

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

No branches or pull requests

4 participants
@brianmcmichael @cristidas @julienmartinlevrai and others