docs(adr): ADR-007: Address generation of chain link proof

[dadamu]

Description

References: #724

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

• included the correct type prefix in the PR title
• added ! to the type prefix if API or client breaking change
• targeted the correct branch (see PR Targeting)
• provided a link to the relevant issue or specification
• followed the guidelines for building modules
• included the necessary unit and integration tests
• added a changelog entry to CHANGELOG.md
• included comments for documenting Go code
• updated the relevant documentation or specification
• reviewed "Files changed" and left comments if necessary
• confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

• confirmed the correct type prefix in the PR title
• confirmed ! in the type prefix if API or client breaking change
• confirmed all author checklist items have been addressed
• reviewed state machine logic
• reviewed API design and naming
• reviewed documentation is accurate
• reviewed tests and test coverage
• manually tested (if applicable)

[RiccardoM]

Honestly I'm not a big fan if this approach. We should use an interface (and the related serialization strategy - Any) only when there are different implementations that need different data to be created. This was true for AddressData:

• Bech32AddressData requires a prefix and a value
• HexAddressData requires only a value
• Base58AddressData requires only a value

In this case, each address generation strategy does not require anything else aside from the public key, which is passed to the GenerateAddress method itself. For this reason, I think the best alternative is to simply add a new method to the AddressData interface instead:

// AddressData is an interface representing a generic external chain address
type AddressData interface {
    proto.Message

    // Validate checks the validity of the AddressData
    Validate() error

    // GetValue returns the address value
    GetValue() string

    // GetAddressGenerationStrategy returns the address generation strategy to be used
    GetAddressGenerationStrategy() AddressGenerationStrategy

    // VerifyAddress verifies that the given address is associated with this address data
    VerifyAddress(address []byte) (bool, error)
}

As you can see, I've also changed VerifyPubKey to VerifyAddress. Now the flow should become:

1. generate the proper address using the associated strategy;
2. verify that the address matches the address data value.

address, err := GenerateAddress(b.GetAddressGenerationStrategy(), key)
if err != nil {
    return err
}

valid, err := addressData.VerifyAddress(bz)
if err != nil {
    return err
}

The GenerateAddress is going to be a public function that simply takes an AddressGenerationStrategy, a public key and generates the address properly:

func GenerateAddress(strategy AddressGenerationStrategy, pubKey cryptotypes.PubKey) (address []byte, err error) {
    switch strategy { 
        case StrategyRipemd160:
            return generateRipemd160Address(pubKey)
        // ...
    }

This implementation will also require to change all the AddressData instances constructors to allow passing an (optional) address generation strategy

[leobragaz]

I'll wait for this to be included in the ADR and then review it. Notify me when ready @dadamu.

[RiccardoM]

which might apply the supported address format but with the unsupported algorithm

I was wondering: do we have such examples?

[dadamu]

elrond address is a bech32 format, but it encode public key into bech32 directly instead of ripemd160(sha256(pubKey)) as cosmos ecosystem.

[RiccardoM]

@dadamu @bragaz I've completely reviewed this ADR as I think there might be an easier way to go to support different algorithm strategies and encodings.

[RiccardoM]

Instead of requesting the developers to put the hex-encoded address bytes here, we can also allow them to put the already-encoded address using the specified encoding algorithm. Then, inside the Validate check we simply:

1. generate the addresss bytes from the public key;
2. encode the address using the provided encoding method;
3. check if the resulting encoded address is equals to the provided value

[dadamu]

I think we should provide the prefix field since Polkadot base58 address format has prefix.
References:
https://wiki.polkadot.network/docs/learn-accounts#address-format

[leobragaz]

I've nothing against this, since it brings more support for other chains.
Thoughts? @RiccardoM

[RiccardoM]

Nothing against, just made it optional though