Bech32Address encoding/decoding (might be) wrong

[ivrusuboca]

While testing the Java and trust core versions of the APIs that connect to the Binance DEX testnet, I noticed that they differ in generating the bech32 address needed to connect to fetch the account information - for example, via : /api/v1/account/{address} .

Further analysis by comparing the Java and C++ implementations resulted in concluding that the C++ one might be faulty.

My understanding is that both implementations are for https://en.bitcoin.it/wiki/BIP_0173 . I have not read these specs entirely ( will do that a bit later ) .

This ticket is to point out that : (1) the Java and C++ implementations lead to different bech32 addresses for the same private key, and (2) the Java implementation seems to be accepted by Binance DEX testnet .

Will follow with a pull request shortly .

[vikmeup]

BIP173 is SegwitBech32 address and Bech32 are two different formats.

Bech32Address follows Cosmos, Binance Chain implementation and Segwit Bech32 only used for the Bitcoin and Litecoin address generation

Please provide (1), we have seen any issues on that side, we were running binance chain on iOS/Android for about two months, works well.

[ivrusuboca]

Thank you for the details. It might be then that I am generating the address the wrong way via the Swift-based API .

Before providing the examples, it seems the below code has a problem with the method std::string Bech32Address::string() of Bitcoin/Bech32Address.cpp.

If I remove the line enc.push_back(witnessVersion); (and I adjust its associated ::decode accordingly ) , then the address I am getting is the same like the Java one .

Here is how I am generating the address :

      let pk = "this is my Binance DEX testnet pk"

      let privateKey = PrivateKey(data: Data(hexString: pk)!)!
      let publicKey = privateKey.getPublicKeySecp256k1(compressed: true)

      let address = Bech32Address(hrp: .binanceTest, publicKey: publicKey)
      NSLog("address : \(address) ");

May I ask kindly you whether the above code is the correct one to generate the Binance DEX address ?

It is true that , if I make the changes in Bech32Address.cpp in order to get the desired address, then lots of test cases are failing - which might suggest my changes might not be quite right.

[vikmeup]

@ivrusuboca use TendermintAddress - this is what you are looking for

[ivrusuboca]

Argh ! True. It works that way. Thank you very much ! I will close this ticket then.

[vikmeup]

@alejandro-isaza as a suggestion to rename Bitcoin::Bech32Address to Bitcoin::SegwitBech32Address to avoid confusion.

[alejandro-isaza]

Will do

[hewigovens]

@alejandro-isaza as a suggestion to rename Bitcoin::Bech32Address to Bitcoin::SegwitBech32Address to avoid confusion.

@vikmeup how about keep Bitcoin::Bech32Address and rename TendermintAddress to Cosmos::Bech32Address? TendermintAddress is confused for me and SegwitBech32Address is a bit long

[ivrusuboca]

What confused me, originally, was that the Java API is using this method to get the address :

    public static String getAddressFromECKey(ECKey ecKey, String hrp) {
        byte[] hash = ecKey.getPubKeyHash();
        return Bech32.encode(hrp, convertBits(hash, 0, hash.length, 8, 5, false));
    }

As such, I went ahead trying to use Bech32Address instead of TendermintAddress , although the latter one I was already using for the purpose of populating the trade order's root id and the sender fields ...