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

Zero-padding of Address#toString() #1131

Closed
avcdsld opened this issue Aug 26, 2021 · 3 comments · Fixed by #1214
Closed

Zero-padding of Address#toString() #1131

avcdsld opened this issue Aug 26, 2021 · 3 comments · Fixed by #1214
Assignees
Labels
Feedback Improvement Language Breaking Change Breaks Cadence contracts deployed on Mainnet

Comments

@avcdsld
Copy link

avcdsld commented Aug 26, 2021

Instructions

When you execute Address#toString() in Cadence, it does not return a zero-padded result.
If the length of Address is fixed, I think it should be zero-padded result.

Problem

When I used Cadence Script to get metadata containing address information as {String:String} type value and compared it with a string, I could not check that it was the same address.

Steps to Reproduce

CADENCE: V0.18.0

  let addr: Address = 0x004b19d1e7c70d25
  log(addr.toString())
  // -> "0x4b19d1e7c70d25"
@turbolent
Copy link
Member

This works as expected.

The value-size of the address is fixed to 64 bit, but there is no fixed size on the string representation of the address. Leading zeros make no difference, e.g. just like there is no difference in the decimal representation of number, e.g. 00000001 being equivalent to 1, 0x00000001 and 0x1 are equivalent.

@jordanschalm
Copy link
Member

jordanschalm commented Oct 15, 2021

Ran into an issue with this behaviour recently as well.

Given that leading-zero addresses are an exception rather than the rule, this behaviour doesn't offer much benefit in my opinion (eg. we aren't saving space in displayed addresses). I'm pretty sure this was originally added before Flow's address generation was solidified, when early emulator tools generated addresses like 1, 2, 3... (with lots of leading zeros).

Currently, this trimmed representation isn't supported consistently. For example:

Unless there's a significant benefit of the trimmed addresses that I'm missing, I feel like the simplest way to make this consistent across all the Flow & community tools would be to use the non-trimmed representation in Cadence as well. (Seems like it would be a one-liner)

@jordanschalm jordanschalm reopened this Oct 15, 2021
@turbolent turbolent removed their assignment Oct 15, 2021
@turbolent
Copy link
Member

This should be indeed a simple change, my opposition to it was mostly because of the rationale, i.e. the leading zeros being unnecessary.

I always assumed that we would want the simplest form, but if most of the tooling behaves differently, then I would also prefer consistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feedback Improvement Language Breaking Change Breaks Cadence contracts deployed on Mainnet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants