LibertyDSNP / eth-sdk-ts

TypeScript SDK for DSNP on Ethereum
Apache License 2.0
9 stars 1 forks source link

Ensure we fetch registrations using Ethereum checksum address #133

Closed shannonwells closed 3 years ago

shannonwells commented 3 years ago

Problem

There was a bug whereby if we passed in the regular Ethereum address, fetching registrations would fail because it's expecting the checksummed version.

Kind of part of #178933068

Solution

Call ethers.utils.getAddress to ensure we use checksum address version in getRegistrationsByWalletAddress to ensure we provide the correct version of the address. with @wilwade , @acruikshank

Double Checks:

Change summary:

Steps to Verify:

Tests should all pass. If you run a local hardhat node and deploy contracts, the following test code output the registration twice:

import {ethers} from "ethers";
import * as sdk from "@dsnp/sdk";

const main = async () => {
  const providerUrl = "http://localhost:8545"

  const account = "0xa0ee7a142d267c1f36714e4a8f75612f20a79720"
  const accountAlt = "0xa0Ee7A142d267C1f36714E4a8F75612F20a79720"

  const pk = "0xdf57089febbacf7ba0bc227dafbffa9fc08a93fdc68e1e42411a14efcf23656e"
  const provider = new ethers.providers.JsonRpcProvider(providerUrl);
  const signer = new ethers.Wallet(pk, provider);

  sdk.setConfig({provider, signer})
  const res = await sdk.handles.createRegistration(account, "tomasina")
  console.log({res})

  const result = await sdk.core.contracts.registry.getRegistrationsByWalletAddress(account)
  console.log({result})

  const resultAlt = await sdk.core.contracts.registry.getRegistrationsByWalletAddress(accountAlt)
  console.log({resultAlt})
}

main();
rosalinekarr commented 3 years ago

@shannonwells, just an fyi, this is the bug I originally open for this when we found the problem: https://www.pivotaltracker.com/story/show/179211543

I went ahead and assigned it to you and marked it as finished since this PR is up for review