refactor(agglayer): refactor EthAddress and EthAddressFormat#2622
Open
partylikeits1983 wants to merge 2 commits intoagglayerfrom
Open
refactor(agglayer): refactor EthAddress and EthAddressFormat#2622partylikeits1983 wants to merge 2 commits intoagglayerfrom
EthAddress and EthAddressFormat#2622partylikeits1983 wants to merge 2 commits intoagglayerfrom
Conversation
EthAddress and EthAddressFormat
EthAddress and EthAddressFormat EthAddress and EthAddressFormat
bobbinth
reviewed
Mar 17, 2026
| /// and the Ethereum address format when constructing CLAIM notes or calling the AggLayer Bridge | ||
| /// `bridgeAsset()` function. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] | ||
| pub struct EthAccountIdFormat(EthAddressFormat); |
Contributor
There was a problem hiding this comment.
Question: why not do this as just EthAccountIdFormat(AccountId) - i.e., internal representation would be just AccountId, but we could convert to/from Ethereum address.
mmagician
reviewed
Mar 18, 2026
Collaborator
mmagician
left a comment
There was a problem hiding this comment.
I find the proposed naming distinction confusing: I often refer to Ethereum address as "account id" (maybe this is just me, it's probably not technically precise language), "account hex" or similar.
If we proceed with this refactoring then I'd much rather have the distinction very clear, maybe something like EthEmbeddedAccountId, or just EmbeddedAccountId (in eth_types module, so the import is clear eth_types::EmbeddedAccountId).
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #2357
Description
The previous
EthAddressFormattype served dual purposes: representing both real Ethereum addresses (e.g., origin token contract addresses, bridge-out destinations) and MidenAccountIdvalues encoded in the 20-byte Ethereum address format (bridge-in destinations). This PR separates these concerns into two distinct types:EthAddressFormat(previouslyEthAddress) — A plain 20-byte Ethereum address. Used for origin token addresses, bridge-out destination addresses, and any context requiring a real EVM address. Providesnew(),from_hex(),as_bytes(),into_bytes(),to_hex(), andto_elements().EthAccountIdFormat(previouslyEthAddressFormat) — A newtype aroundEthAddressFormatfor the bridge-in (CLAIM) flow, where a MidenAccountIdis encoded as[0, 0, 0, prefix, suffix]. Addsfrom_account_id()andto_account_id()conversion methods. Converts to/fromEthAddressFormatviaFromimpls.Breaking Changes
EthAddressFormatno longer hasfrom_account_id()orto_account_id()methods. UseEthAccountIdFormatforAccountId↔ Ethereum address conversions.From<AccountId> for EthAddressFormatimpl has been removed. UseEthAccountIdFormat::from(account_id)instead.File Structure
The old
address.rshas been split into:eth_address_format.rs—EthAddressFormat+AddressConversionErroreth_account_id_format.rs—EthAccountIdFormat(imports frometh_address_format)