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

[AIP-40][Address Standard v1] #178

Closed
thepomeranian opened this issue Jun 21, 2023 · 13 comments
Closed

[AIP-40][Address Standard v1] #178

thepomeranian opened this issue Jun 21, 2023 · 13 comments

Comments

@thepomeranian
Copy link
Collaborator

AIP Discussion

Summary

This standard defines the following:

  • What format APIs should return addresses in.
  • What formats APIs should accept addresses in.
  • How addresses should be displayed.
  • How addresses should be stored.

Out of Scope

Prior discussion has centered around ways to differentiate between addresses, public / private keys, and other representations of data as hex. There has also been discussion around a more compact representation of addresses. This standard is not related to these discussions (which are akin to a v2 standard), it only aims to standardize our existing approach (v1).

Motivation

Each account / object on the Aptos blockchain is identified by a 32 byte account address. For internal uses, addresses are represented as just that, using a 32 byte sequence. However when transmitting and displaying addresses it is common to use a hex representation. As it stands today there is no standard that defines how these addresses should be represented in each context. Since there are multiple technically correct ways to represent an address, this leads to a fractured ecosystem in which different APIs, tools, and products represent and accept addresses differently. This can lead to issues with querying on-chain data and submitting correct transactions.

Read more about it here: https://github.com/aptos-foundation/AIPs/blob/main/aips/aip-40.md

@sirouk
Copy link

sirouk commented Jun 26, 2023

I agree. There should be a standard. We should adopt and implement now rather than later when it would be more impactful.

@alinush
Copy link
Contributor

alinush commented Jun 27, 2023

In the AIP, what do you mean by:

there is potential for malicious actors to display addresses that look like special addresses but are not, e.g. 0x0{62}1 vs 0x0{63}1.

Can you clarify in the AIP what are the {62} and {63} thingies denoting?

@banool
Copy link
Contributor

banool commented Jun 27, 2023

I'm just using regex notation. So 0{62} would mean 62 zeroes. I don't think I'm explaining the problem with SHORT format for non-special addresses very well, I can rewrite a bit.

@ruuda
Copy link

ruuda commented Jun 27, 2023

We previously ran into some issues with https://explorer.aptoslabs.com/validators/delegation not displaying the right names, caused by some places using the LONG format and some places using the SHORT format (and some places checking for string equality rather than equality of the byte array, I assume). It would be nice to drop the SHORT format and use the LONG format consistently everywhere.

@banool
Copy link
Contributor

banool commented Jun 27, 2023

For the special (where "special" is defined in the AIP) addresses like 0x1, people generally expect them to be formatted in the SHORT format. What do you think?

@AntoTG
Copy link

AntoTG commented Jun 27, 2023

At Stakely, we support this proposal. In the previewnet, we encountered some inconsistencies with a wallet that started with two zeros (0x00{62}). Depending on the API method, Petra, CLI commands, or configuration files used, this wallet sometimes displayed as {64}, sometimes as 0x{64}, and sometimes 0x00{62}. This inconsistency was somewhat confusing to realize. A key point of confusion for us was:

Upon executing the command:

aptos stake initialize-stake-owner --operator-address 0x{64}

And then, when we queried the API, the wallet 0x{62} was shown in the operator_address field. Our initial thought was that there might be a mistake due to the mismatch - we assumed that we had input the wrong wallet, as one does not typically keep track of whether the wallet contains 62 or 64 characters. Eventually, we understood that there was no issue and that the wallet addresses were equivalent.

image

Another aspect we believe is crucial to standardize relates to the validator configuration files (e.g., validator-identity.yaml). In these files, the "account_address" field is not preceded by 0x, but it shows as {64}. We are wondering if this proposal also aims to standardize the address format in the node configuration files?

@banool
Copy link
Contributor

banool commented Jun 27, 2023

Thanks for the feedback! Yes we'd love to standardize the address format in the node configuration files. Essentially how this would look is the following:

  • The parser for the configuration files would accept addresses as per the standard (permissive).
  • The examples and generated configs would all be changed to use LONG

@A7610605
Copy link

Fully support this! Before mainnet launch, I generated a vanity address with 8 leading zeros for fun and use it as node operator address(guguru). I've encountered a lot of confusing scenarios or mismatch.
For example, In the schemas of the API spec, there are Address and HexEncodedBytes. And the description of Address is "sometimes shortened by stripping leading 0s, and adding a 0x. " The API accepts SHORT and output SHORT addresses. However, the aptosnames API does not accept SHORT despite the documentation of aptosnames is use Address. This causes the names with leading 0 addresses to not display on the official explorer for a while.
And the third-party explorer apscan is further odd, as it only accepts SHORT and cannot use LONG, together with aptosnames API returns LONG, which makes it impossible to use aptosname to query addresses with leading 0.
In addition, the current API doesn't appear to accept all SHORT addresses without leading 0x. Is it necessary to make changes to special addresses like in the AIP?

@banool
Copy link
Contributor

banool commented Jun 28, 2023

Yeah some changes will be required. Where changes will be backwards compatible, we will make them. If not, we will make changes for future iterations. I'll update the ecosystem impact section of the AIP soon with this information!

@davidiw
Copy link
Contributor

davidiw commented Jul 26, 2023

Do we have any applications that actually output short without "0x"? and similarly do any applications actually output long without "0x"? It might be good to at least in the discussion mention a few applications that went too permissive.

@banool
Copy link
Contributor

banool commented Jul 26, 2023

Some examples of applications that output long without 0x off the top of my head include:

  • The CLI (e.g. for accounts, for the commands for generating the node config, etc).
  • The API in some cases (I'll do more research on this).

I can enumerate some more and add them to the motivation in the AIP.

As for outputting short without 0x, not sure. I know we had issues with this in the explorer on the ingestion side but idk where those addresses came from in the first place. I'll investigate.

@banool
Copy link
Contributor

banool commented Aug 1, 2023

More information about how we'll integrate this into the TS SDK here: #193 (comment).

@banool
Copy link
Contributor

banool commented Aug 15, 2023

Through consultation with the working group for the AIP we have clarified how reference implementations should handle parsing of account address strings. In particular, it explicitly lays out the preferred parsing behavior while allowing more relaxed behavior for the sake of migration. See the updated AIP.

With these changes in mind, we have landed AccountAddress to the v2 TS SDK. The AIP has been updated to refer to this code as the reference implementation.

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

8 participants