fuels-ts icon indicating copy to clipboard operation
fuels-ts copied to clipboard

fix: internalize `ethers` functionality and remove dependency

Open maschad opened this issue 1 year ago • 6 comments

Closes https://github.com/FuelLabs/fuels-ts/issues/2153

ethers is now removed from:

  • account
  • crypto

As a result there are more ethers functions being internalized:

  • ripemd160
  • pbkdf2
  • hmac
  • dataSlice
  • encodeBase58
  • decodeBase58
  • getBytes

This can be merged as a follow up to https://github.com/FuelLabs/fuels-ts/pull/2152

maschad avatar Apr 22 '24 14:04 maschad

✨ A PR has been created under the rc-2130 tag on fuels-wallet repo. https://github.com/FuelLabs/fuels-wallet/pull/1242

fuel-service-user avatar Apr 22 '24 14:04 fuel-service-user

The getNetwork() work here appears to be small compared to the rest, making the PR title slightly misleading.

Should we split the PR into two, dealing with the getNetwork() in a subsequent PR while re-shaping this one to be concerned only with replacing ethers utilities?

That's a good point @arboleya I'll do that.

I've converted this to draft until https://github.com/FuelLabs/fuels-ts/pull/2152 is merged, but it can still be reviewed, the tests will fail as I have removed the ethers dep in accounts.

maschad avatar Apr 23 '24 17:04 maschad

Great work @maschad, another good step towards decoupling from ethers.

What was the reasoning behind implementing getBytes? As it looks exactly the same as our own arrayify?

Thanks! There are some additional parameters included in the function signature for getBytes that I would need for some use cases I opted not to modify the arrayify to reduce the surface of these changes, but if that's acceptable I can include them in this PR.

maschad avatar May 02 '24 20:05 maschad

IMO that is acceptable here, I'd rather have one point of entry for byte array conversion. I probably would have done the same when we initially internalised ethers if it didn't work with all the use cases.

danielbate avatar May 03 '24 02:05 danielbate

As discussed, amalgamate the functionality of getBytes and arrayify.

Thanks @danielbate I've updated in 69bf67a

maschad avatar May 06 '24 19:05 maschad

Nice work 💪🏻

Jw, what ethers deps are left after this?

Thanks 😄

Looks like just doc-snippets and abi-coder now

maschad avatar May 10 '24 14:05 maschad

Coverage Report:

Lines Branches Functions Statements
79.53%(-0.13%) 69.79%(+0%) 77.12%(-0.48%) 79.64%(-0.12%)
Changed Files:
Ok File (✨=New File) Lines Branches Functions Statements
🔴 ✨ packages/crypto/src/browser/hmac.ts 0%
(+0%)
0%
(+0%)
0%
(+0%)
0%
(+0%)
🔴 ✨ packages/crypto/src/browser/pbkdf2.ts 0%
(+0%)
100%
(+100%)
0%
(+0%)
0%
(+0%)
🔴 ✨ packages/crypto/src/node/hmac.ts 46.66%
(+46.66%)
0%
(+0%)
0%
(+0%)
46.66%
(+46.66%)
🔴 ✨ packages/crypto/src/node/pbkdf2.ts 46.66%
(+46.66%)
0%
(+0%)
0%
(+0%)
46.66%
(+46.66%)
🔴 ✨ packages/crypto/src/shared/ripemd160.ts 69.23%
(+69.23%)
0%
(+0%)
50%
(+50%)
71.42%
(+71.42%)
✨ packages/utils/src/utils/base58.ts 100%
(+100%)
100%
(+100%)
100%
(+100%)
100%
(+100%)
✨ packages/utils/src/utils/dataSlice.ts 100%
(+100%)
100%
(+100%)
100%
(+100%)
100%
(+100%)

github-actions[bot] avatar May 10 '24 18:05 github-actions[bot]