decentralized-identity / veramo

A JavaScript Framework for Verifiable Data
https://veramo.io
Apache License 2.0
414 stars 130 forks source link

chore: bump ethers #1242

Closed fermentfan closed 9 months ago

fermentfan commented 9 months ago

Still have to fix tests...

fermentfan commented 9 months ago

@mirceanis I have a hard time finding out whether these test results are really showing an error in the system. The generated signatures mismatch from the expected ones, but maybe that is ok as the content changed with the bigint usage?

image

These are the only tests currently failing. Now waiting for release of ethr-did-resolver and ethr-did.

fermentfan commented 9 months ago

I merge next into the branch and switched to the official releases of the 'ethr-did-resolver' & 'ethr-did'. Ready for review!

mirceanis commented 9 months ago

I figured out why the tests are failing there.

ethers infers the transaction type when serializing it. v6 does the inference a little differently and ends up serializing transactions that specify a gasPrice as type 1 where v5 would infer type 0.

Both transaction forms would be accepted by the EVM so it is ok to update the test vectors with the new values.

For completeness, we should also have a test that sets the transaction type to 0 and compares with the previous test vectors:

      it('should sign EthTX using generic signer and specific type', async () => {
        const transaction = Transaction.from({
          to: '0xcE31a19193D4b23F4E9D6163d7247243BAF801C3',
          value: 300000,
          gasLimit: 43092000,
          gasPrice: 20000000000,
          nonce: 1,
          type: 0, // enforce legacy serialization
        })

        const txData = transaction.unsignedSerialized

        const rawTx = await agent.keyManagerSign({
          algorithm: 'eth_signTransaction',
          data: txData,
          encoding: 'hex',
          keyRef: importedKey.kid,
        })

        expect(rawTx).toEqual(
          '0xf869018504a817c800840291882094ce31a19193d4b23f4e9d6163d7247243baf801c3830493e0801ba0f16e2206290181c3feaa04051dad19089105c24339dbdf0d80147b48a59fa152a0770e8751ec77ccc78e8b207023f168444f7cfb67055c55c70ef75234458a3d51',
        )
      })
fermentfan commented 9 months ago

Ahh thanks for spotting that. Amazing! I pushed the changes including the new test now. Let's see what the CI/CD says. At least on my local machine everything is green

codecov[bot] commented 9 months ago

Codecov Report

Attention: 26 lines in your changes are missing coverage. Please review.

Comparison is base (93198bc) 84.99% compared to head (a6a796f) 84.91%. Report is 1 commits behind head on next.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## next #1242 +/- ## ========================================== - Coverage 84.99% 84.91% -0.09% ========================================== Files 167 167 Lines 18001 18031 +30 Branches 2009 2017 +8 ========================================== + Hits 15300 15311 +11 - Misses 2701 2720 +19 ``` | [Files](https://app.codecov.io/gh/uport-project/veramo/pull/1242?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uport-project) | Coverage Δ | | |---|---|---| | [packages/did-provider-key/src/key-did-provider.ts](https://app.codecov.io/gh/uport-project/veramo/pull/1242?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uport-project#diff-cGFja2FnZXMvZGlkLXByb3ZpZGVyLWtleS9zcmMva2V5LWRpZC1wcm92aWRlci50cw==) | `76.92% <100.00%> (ø)` | | | [packages/did-provider-pkh/src/pkh-did-provider.ts](https://app.codecov.io/gh/uport-project/veramo/pull/1242?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uport-project#diff-cGFja2FnZXMvZGlkLXByb3ZpZGVyLXBraC9zcmMvcGtoLWRpZC1wcm92aWRlci50cw==) | `62.72% <100.00%> (ø)` | | | [packages/key-manager/src/types.ts](https://app.codecov.io/gh/uport-project/veramo/pull/1242?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uport-project#diff-cGFja2FnZXMva2V5LW1hbmFnZXIvc3JjL3R5cGVzLnRz) | `100.00% <100.00%> (ø)` | | | [packages/kms-local/src/secret-box.ts](https://app.codecov.io/gh/uport-project/veramo/pull/1242?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uport-project#diff-cGFja2FnZXMva21zLWxvY2FsL3NyYy9zZWNyZXQtYm94LnRz) | `95.65% <100.00%> (-0.10%)` | :arrow_down: | | [...ackages/kms-web3/src/web3-key-management-system.ts](https://app.codecov.io/gh/uport-project/veramo/pull/1242?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uport-project#diff-cGFja2FnZXMva21zLXdlYjMvc3JjL3dlYjMta2V5LW1hbmFnZW1lbnQtc3lzdGVtLnRz) | `68.38% <100.00%> (-0.24%)` | :arrow_down: | | [packages/utils/src/did-utils.ts](https://app.codecov.io/gh/uport-project/veramo/pull/1242?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uport-project#diff-cGFja2FnZXMvdXRpbHMvc3JjL2RpZC11dGlscy50cw==) | `83.56% <100.00%> (-0.05%)` | :arrow_down: | | [packages/did-provider-ion/src/functions.ts](https://app.codecov.io/gh/uport-project/veramo/pull/1242?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uport-project#diff-cGFja2FnZXMvZGlkLXByb3ZpZGVyLWlvbi9zcmMvZnVuY3Rpb25zLnRz) | `94.13% <50.00%> (-0.02%)` | :arrow_down: | | [packages/key-manager/src/key-manager.ts](https://app.codecov.io/gh/uport-project/veramo/pull/1242?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uport-project#diff-cGFja2FnZXMva2V5LW1hbmFnZXIvc3JjL2tleS1tYW5hZ2VyLnRz) | `93.61% <80.00%> (-0.06%)` | :arrow_down: | | [packages/kms-local/src/key-management-system.ts](https://app.codecov.io/gh/uport-project/veramo/pull/1242?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uport-project#diff-cGFja2FnZXMva21zLWxvY2FsL3NyYy9rZXktbWFuYWdlbWVudC1zeXN0ZW0udHM=) | `90.32% <95.45%> (+0.10%)` | :arrow_up: | | [.../key-manager/src/abstract-key-management-system.ts](https://app.codecov.io/gh/uport-project/veramo/pull/1242?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uport-project#diff-cGFja2FnZXMva2V5LW1hbmFnZXIvc3JjL2Fic3RyYWN0LWtleS1tYW5hZ2VtZW50LXN5c3RlbS50cw==) | `61.01% <12.50%> (-6.26%)` | :arrow_down: | | ... and [2 more](https://app.codecov.io/gh/uport-project/veramo/pull/1242?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uport-project) | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.