Support 20 bytes address

[lxfind]

The current address is hard-coded to be 16 bytes. This works well for Diem, but could be insufficient for other systems. A common requirement is 20 bytes.
This PR adds a feature that gates the size of the address to be either 16 or 20 bytes, and change everywhere this is used to be using the right length.

Basically redo diem/diem#10015 since the repo moved.

[sblackshear]

Accepting here since this was approved on the Diem side.

[mkurnikov]

Hi, in our Pontem Network fork of the Move compiler, we use 32 bytes as address length https://github.com/pontem-network/diem/blob/v1.5.1-pre/language/move-core/types/src/account_address.rs#L20.
Is it possible to add even more configurability to this value?

[sblackshear]

/land

[sblackshear]

Hi, in our Pontem Network fork of the Move compiler, we use 32 bytes as address length https://github.com/pontem-network/diem/blob/v1.5.1-pre/language/move-core/types/src/account_address.rs#L20. Is it possible to add even more configurability to this value?

@mkurnikov Yes, this is possible. There's some discussion about using const generics to allow arbitrary address lengths in diem/diem#10015, but the conclusion is that this would be a significant refactoring.

To allow 32 byte addresses, the quickest path would be to send another PR mimicking this one that adds an address32 feature. But would also be very happy (happier, in fact!) to support a PR performing the refactoring required to enable full configurability.

[bors-libra]

💔 Test Failed - ci-test

[sblackshear]

/land

[bors-libra]

@sblackshear ❗ This PR is still missing approvals, unable to queue for landing

[sblackshear]

/land

[bors-libra]

💔 Test Failed - ci-test

[lxfind]

@tnowacki There are unit test failures with this PR, but I am unable to reproduce it locally. All tests pass on my Mac. Not sure what's going on? Any insights on what might be happening here?

[tnowacki]

@lxfind

1. This test is weird and duplicates a module for no reason
2. Let me check you your branch and take a look

[tnowacki]

Does this add the feature all the time? I'm confused

[lxfind]

No it just defines the feature. To enable it address20 needs to be adde to default (or explicitly specified when adding the move-core-types dependency)

[tnowacki]

No idea whats up, rerunning the jobs to see if it was just flakiness? As in general flakiness or just that test. Or if that test is really broken somehow?

[tnowacki]

Same test again. Going to try running all tests on my mac and see if maybe it is some config poisoning thing or something between tests

[tnowacki]

I'm at a loss. Maybe @tzakian has some CLI related ideas?

[tnowacki]

Another possible thought, is your branch on your fork up to date? That is, are main on your fork and main on diem/move equivalent?

I have seen some CI flakiness when those aren't in sync

[lxfind]

Another possible thought, is your branch on your fork up to date? That is, are main on your fork and main on diem/move equivalent?

I have seen some CI flakiness when those aren't in sync

Yeah I rebased to main today.

[tnowacki]

Maybe @bmwill knows someone who can help debug this?

[tnowacki]

Only other thing I could imagine, just throwing debugging ideas out here, switch the address from 0x3 to something else for this test (maybe 0x1) just to rule out a bug with the address parsing code

[lxfind]

OK. So I reverted my commit and added some dummy changed (empty lines), and the test still failed. So pretty sure it's not related to my change.

[tnowacki]

As a stopgap measure, can you see how the test looks with renaming module A? Or changing the address? There is no reason whatsoever that the two modules need the same name between the two test files

[lxfind]

As a stopgap measure, can you see how the test looks with renaming module A? Or changing the address? There is no reason whatsoever that the two modules need the same name between the two test files

Renaming it does fix the test failure. But it also kind of changed the semantics of the test (the test seems to be testing the duplicate module name on purpose?)

[lxfind]

I edited the test file so that it can pass.
Filed issue #22 to track the broken test.

[tnowacki]

/land