feat: native bigint support, modernization of sdk

[koraykoska]

This is part of the sdk upgrade grant.

Updates

• Removed tsdx (as it's not maintained anymore)
• Added native BigInt to BigIntish and use native BigInt for calculations internally (rationale: Native BigInt is orders of magnitude faster than JSBI and more concise, easier to read and code)
• Update test suite folder structure to be easier to navigate the source code

Important backwards compatibility notes

The updates in this PR are fully source-code compatible and non-breaking. But explicit < es2020 projects will not be able to compile it.
So, if people use a modern setup, they won't have to change anything in their existing code but can start using native BigInt (and even if not will feel a performance difference).
Because of the above, I propose making a major version bump anyways. That way, users will need to explicitly opt-in to the new core-sdk, but if they do they don't have to update any source code.

BigInt reduces verbosity-based bugs and helps code to be concise and easier to read. So people will be able to read through the source code going forward more easily and possibly make better contributions because of that.

[stale]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[koraykoska]

@cbachmeier @just-toby

[just-toby]

bigint changes look good to me. can we do the tests reorganization in a separate PR ?

[zzmp]

This looks great, thank you!, but because it's a foundational repo I think we should be careful with big changes. Can you justify changing from esm to cjs? I'm worried that this change may break downstream users, especially because the compilation and distribution are changed in this PR.

Also, because it's a foundational repo, Is there any way you could break this up into discrete PRs for each functional change (or any reason that you can't): esm->cjs, add jest, native bigint support?

[koraykoska]

@zzmp

1. Breaking up the PR into multiple defeats the purpose of the fork in the Uniswap foundation Github. We decided it's better to have the scratchpad there and submit PRs for every milestone. We understand that it can take time to get proper reviews and hence wanted to combine it as much as possible instead of sitting on multiple open PRs before being able to continue. Splitting up the PR again after having done proper project management on the foundation repo would be a waste of everyone's time.

2. When we dropped the deprecated build tools library, we had to choose between commonjs and es modules. As far as I could see, commonjs compiles are usable with both commonjs and es modules. Whereas the other way round is only compatible with es modules (not relevant for typescript but still relevant for plain JS).

If you have reasons to switch to es modules only and are ok with support for older JS projects being dropped I can go ahead.

[elee1766]

could you remove the depdnency on @ethersproject/address

[Florian-S-A-W]

could you remove the depdnency on @ethersproject/address

I'll look into it, it's only used once in the project I think.

[grabbou]

Looks good to me, but I don't have experience working with BigInt's to comment on the actual changes. I had a look from the infrastructure and packaging perspective, since this is what I am working on on a daily basis.

[krzkaczor]

@koraykoska, any ETA on this? sdk-core is quite a widespread package in the ecosystem, and no native bigint support is a problem.

[koraykoska]

@krzkaczor Waiting for the final reviews.

[Florian-S-A-W]

Looks good to me.

[koraykoska]

@just-toby Please re-run the linter workflow. I had to enable push permissions for github actions

[koraykoska]

@zzmp @JFrankfurt Can someone please re-run the linter workflow please?

[koraykoska]

@just-toby @zzmp @JFrankfurt Please fix the permission issue with the linter and re-run it :)

[stale]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[Florian-S-A-W]

Yes this is still relevant. This can be merged after rerunning the actions.

[shuhuiluo]

Yes this is still relevant. This can be merged after rerunning the actions.

How hard is it for them to run the actions? Lol.

[maxandron]

Yes this is still relevant

[koraykoska]

@just-toby @zzmp @cbachmeier @JFrankfurt @grabbou

[koraykoska]

Please don't let this go over more than 1 year