hicommonwealth / edgeware-lockdrop

Lockdrop contracts
GNU General Public License v3.0
23 stars 16 forks source link

Interpretation of decoding using bs58, stripping its last 6 bits, and re-encoding with Polkadot.js keyring encode instead of bs58's encode #33

Open ltfschoen opened 4 years ago

ltfschoen commented 4 years ago

When I run this test https://github.com/hicommonwealth/edgeware-lockdrop/blob/master/test/2-lockdrop.spec.js#L107 it fails with error on line assert.equal(elt[0], constants.FIXTURES[inx].base58Address);:

       should ensure base58 encodings are valid to submit:

      AssertionError: expected '2afa34ee0f034817963d83845920938c1d23bd7f7d146f588ff0e0f608fd3b6d' to equal '5HimYxXdszMgAbPQD49kLbgaBb274ubQpRNDJmZD4fA7KrJq'
      + expected - actual

      -2afa34ee0f034817963d83845920938c1d23bd7f7d146f588ff0e0f608fd3b6d
      +5HimYxXdszMgAbPQD49kLbgaBb274ubQpRNDJmZD4fA7KrJq

Shouldn't we be encoding the value elt[0] like the following?

const strippedKey = elt[0];
const bytes = Buffer.from(strippedKey, 'hex');
const encodedKey = bs58.encode(bytes);
assert.equal(encodedKey, constants.FIXTURES[inx].base58Address);

Also, why are we decoding using bs58.decode ..., but then encoding using Polkadot.js here https://github.com/hicommonwealth/edgeware-lockdrop/blob/master/helpers/lockdropHelper.js#L272 with const encoded = keyring.encodeAddress(key);, but not using theencoded` variable anyway?

And why are we removing the last 6 bits from the edgewareAddr public key (without preceding 0x) here https://github.com/hicommonwealth/edgeware-lockdrop/blob/master/helpers/lockdropHelper.js#L186, is this the checksum mentioned here https://github.com/paritytech/substrate/wiki/External-Address-Format-(SS58)? It causes the value to be shortened to '2afa34ee0f034817963d83845920938c1d23bd7f7d146f588ff0e0f608fd3b6d' instead of '2afa34ee0f034817963d83845920938c1d23bd7f7d146f588ff0e0f608fd3b6d4ec0be', so we can't encode it back again using the approach mentioned above using bs58.encode ...

Also, why do we decode with bs58.decode here https://github.com/hicommonwealth/edgeware-lockdrop/blob/master/test/2-lockdrop.spec.js#L78, instead of with Polkadot.js's decodeAddress?

const { decodeAddress } = require('@polkadot/util-crypto');
...
      return await lockdrop.lock(TWELVE_MONTHS, `0x${decodeAddress(constants.FIXTURES[inx].base58Address, true).toString('hex')}`, (Math.random() > 0.5) ? true : false, {
ltfschoen commented 4 years ago

I understand it better now, here's a Gist where I used either bs58 or Polkadot's util-crypto library to achieve the same decoding/encoding https://gist.github.com/ltfschoen/f98e5af52dd5ef60e87e87f143d70625

I got it to work in commits of this PR https://github.com/DataHighway-DHX/mining/pull/6