blockchain-lab-um / masca

Snap for managing VCs and DIDs in MetaMask
https://masca.io
Apache License 2.0
53 stars 16 forks source link

[bug](snap): Casting global ethereum object to any obfuscates `resolveDid` error #547

Closed radleylewis closed 7 months ago

radleylewis commented 8 months ago

Describe the bug

Overview Within the networks each new Web3Provider has its first argument as the global ethereum object. However, this obfuscates a bug that occurs when attempting to resolve a did that has been amended.

// networks code from the veramo agent
const networks = [
      {
        name: 'mainnet',
        provider: new Web3Provider(ethereum as any),
      },
      {
        name: '0x05',
        provider: new Web3Provider(ethereum as any),
      },
      {
        name: 'goerli',
        provider: new Web3Provider(ethereum as any),
        chainId: '0x5',
      },
      {
        name: 'sepolia',
        provider: new Web3Provider(ethereum as any),
        chainId: '0xaa36a7',
        registry: '0x03d5003bf0e79c5f5223588f347eba39afbc3818',
      },
    ]

For example, when resolving a did which has been amended (e.g. has had a serviceEndpoint added to the didDoc), it will not resolve, but will instead throw an error of provider.getLogs() is not defined/no such function.

Below is a screenshot of the specific line of code (see: ethr-did-resolver/src/resolver.ts):

image

To Reproduce

  1. create a did/generate a new did
  2. update the did for a change/amendment (e.g. add a serviceEndpoint to the didDoc
  3. attempt to resolve the changed/amended did with resolveDID which will fail as outlined above.

Expected behavior

The resolveDID rpc call should return the didDoc associated with the did, along with any amendments.

martines3000 commented 8 months ago

Hi. Thanks for the detailed description.

Could you provide me with 1 did that was amended, so I can use it to debug the code ?

Thank you.

radleylewis commented 8 months ago

Sure thing! The following did won't resolve with masca's resolveDid rpc call:

did:ethr:0xaa36a7:0x0371ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d

The resolved didDoc should look like the below (I am able to resolve it from a nodejs Veramo configuration using ethr-did-resolver (NOTE: see service field):

 {
  id: 'did:ethr:0xaa36a7:0x0371ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d',
  verificationMethod: [
    {
      id: 'did:ethr:0xaa36a7:0x0371ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d#controller',
      type: 'EcdsaSecp256k1RecoveryMethod2020',
      controller: 'did:ethr:0xaa36a7:0x0371ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d',
      blockchainAccountId: 'eip155:11155111:0xf6CE77B0D921940726cC8742c03914C9312a79b3'
    },
    {
      id: 'did:ethr:0xaa36a7:0x0371ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d#controllerKey',
      type: 'EcdsaSecp256k1VerificationKey2019',
      controller: 'did:ethr:0xaa36a7:0x0371ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d',
      publicKeyHex: '0371ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d'
    }
  ],
  authentication: [
    'did:ethr:0xaa36a7:0x0371ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d#controller',
    'did:ethr:0xaa36a7:0x0371ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d#controllerKey'
  ],
  assertionMethod: [
    'did:ethr:0xaa36a7:0x0371ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d#controller',
    'did:ethr:0xaa36a7:0x0371ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d#controllerKey'
  ],
  service: [
    {
      id: 'did:ethr:0xaa36a7:0x0371ab768c9d855e3b5c2bd94ba425d9105e122c1b424467f79df53e3175b30b3d#service-3',
      type: 'DIDCommMessaging',
      serviceEndpoint: 'https://ddc1.serveo.net/api/ssdt/messaging'
    }
  ],
  '@context': [
    'https://www.w3.org/ns/did/v1',
    'https://w3id.org/security/suites/secp256k1recovery-2020/v2',
    'https://w3id.org/security/v3-unstable'
  ]
}
hylim-tech-lover commented 8 months ago

@martines3000 I faced the similar issue and in addition to the information that @radleylewis have provided, I have constructed a simply conditional flowchart to illustrate on the behavior of DID resolver configuration that I have observed along the troubleshooting process.

image

Hope it could give more insights and fast-tracked the root cause analysis process

martines3000 commented 7 months ago

Hey @hylim-tech-lover @radleylewis.

I created a new release (v1.2.0-beta.2) available here: https://www.npmjs.com/package/@blockchain-lab-um/masca/v/1.2.0-beta.2

You will need MetaMask Flask for this version. You can test it on our beta dapp.

The issues should be resolved, if this is not the case, please let me know. Thank you.

hylim-tech-lover commented 7 months ago

Hi @martines3000 thanks for heads-up! Unfortunately I couldn't find any place to test the resolveDid function from beta dapp that you mentioned.

However, I did use Masca SDK in my DAPP and managed to resolve the did successfully now! It is safe to say it does work. Cheers! 😄

Code snippet of Masca SDK

      console.log(`Connecting to MASCA SNAP app`);
      const enableResult = await enableMasca(selectedAccount, {
        snapId: 'npm:@blockchain-lab-um/masca',
        version: '1.2.0-beta.2',
      });

      const mascaSnap = enableResult.data.getMascaApi();

      await mascaSnap.setCurrentAccount({
        account: ...,
      });

     const resolveDid = await mascaSnap.resolveDID(...);

     console.log(
       'did resolved result: ',
       resolveDid.data,
     );
...
martines3000 commented 7 months ago

Thank you for helping test Masca and reporting the issue.

I will close this issue now. If you find anything else not working correctly, please notify us 🚀 .

radleylewis commented 7 months ago

Thanks @martines3000 . That's awesome - very much appreciated!