3315 Genesis address format - address size and type change

[abergasov]

Motivation

Closes #3315

Changes

• address moved to separate pkg. probably in future will be as independent library
• address still array of bytes ([24]byte), but implement bech32 algorithm. Additional first byte reserved for contain HumanReadablePart of address (now contain info about networkID - test|main net), so result is [25]byte
• all api endpoints will be changed to use string instead []byte (related pr https://github.com/spacemeshos/api/pull/173)

Test Plan

unit tests

TODO

• [ ] Explain motivation or link existing issue(s)
• [ ] Test changes and document test plan
• [ ] Update documentation as needed

DevOps Notes

• [ ] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources)
• [ ] This PR does not affect public APIs
• [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.)
• [x] This PR does not make changes to log messages (which monitoring infrastructure may rely on)

[dshulyak]

could you please use aliases for such refactoring?

package types

type Address = address.Address

they were created specifically to make such changes reviewable

[abergasov]

why cosmos/bech32 and not one from btcd repo? are they different?

source code almost the same. small difference between const|wrappers, but in generally code the same. it can easy replaced by fixing go.mod

i chose cosmos cause it's approach - to get necessary functions and types, for crypto projects. in case btcd repo - we need to vendor other blockchein implementation, that's look little bit weird for me.

[abergasov]

i am generally surprised by the number of files changed for this PR.

what is the advantage of having a separate pkg for address? in another words, what is the disadvantage of maintaining the flat structure inside common/types?

alias type Address = address.Address in common/type reduce total edited files =)

one of idea was to move address implementation to separate library, which will be included as dependency. in this case i moved to nested pkg. when we will publish as single library - just change imports

[lrettig]

one of idea was to move address implementation to separate library, which will be included as dependency

This is not a terrible idea - do we think other projects will have this dependency? (And would they want to use just an address module without relying on all of go-spacemesh?) E.g., sm-repl, dash/explorer, etc.

[countvonzero]

one of idea was to move address implementation to separate library, which will be included as dependency

This is not a terrible idea - do we think other projects will have this dependency? (And would they want to use just an address module without relying on all of go-spacemesh?) E.g., sm-repl, dash/explorer, etc.

but why? what is the problem with having the impl inside common/types/address.go? the functionality you are providing in the new pkg look highly similar to the old ones. @abergasov

[abergasov]

This is not a terrible idea - do we think other projects will have this dependency? (And would they want to use just an address module without relying on all of go-spacemesh?) E.g., sm-repl, dash/explorer, etc.

there is no strong arguments that we should move it to separate pkg, only this 2:

1. our possibly moving address in separate lib in future
2. current situation in common/types, where we have 25 different files aka 25 different types (there are total 33 files with tests) and some of this files contains lot of code - this look like place where we can add more pkg separation and make it more strict

if you think that it better move back to common/types - ok, i'll do. arguments for separate pkg not very strong

[countvonzero]

This is not a terrible idea - do we think other projects will have this dependency? (And would they want to use just an address module without relying on all of go-spacemesh?) E.g., sm-repl, dash/explorer, etc.

there is no strong arguments that we should move it to separate pkg, only this 2:

1. our possibly moving address in separate lib in future

i think this is a future thing and should be separated from this discussion.

2. current situation in common/types, where we have 25 different files aka 25 different types (there are total 33 files with tests) and some of this files contains lot of code - this look like place where we can add more pkg separation and make it more strict

right. but changing only 1 makes it even more confusing. i'd support if you make a sweeping change later to change all of them. but the code inside each file (tho large) are mostly very simple code... i personally don't think it's worth the effort now.

if you think that it better move back to common/types - ok, i'll do. arguments for separate pkg not very strong

that will be my preference. and worry about separate library when the use case arise.

[dshulyak]

bors try

[bors[bot]]

try

Build failed:

• ci-stage2

[abergasov]

bors try

[bors[bot]]

try

Build failed:

• ci-stage2

[dshulyak]

what about system tests?

[abergasov]

what about system tests?

it fails in same place cluster.Reuse with ctx timeout. Probably node fails to start in case invalid genessis|coinbase accounts

[abergasov]

bors try

[bors[bot]]

try

Build succeeded:

• ci-stage2

[abergasov]

bors try

[bors[bot]]

try

Build failed:

• ci-stage2

[abergasov]

bors try

[bors[bot]]

try

Build succeeded:

• ci-stage2

[abergasov]

bors merge

[bors[bot]]

:-1: Rejected by code reviews

[abergasov]

bors merge

[bors[bot]]

:-1: Rejected by code reviews

[abergasov]

bors try

[bors[bot]]

try

Build succeeded:

• ci-stage2

[dshulyak]

i thought that it was merged. is there any reason why it is still open?

[abergasov]

bors try

[bors[bot]]

try

Build succeeded:

• ci-stage2

[abergasov]

bors merge

[bors[bot]]

Pull request successfully merged into develop.

Build succeeded:

• ci-stage2