js-sdk icon indicating copy to clipboard operation
js-sdk copied to clipboard

Feature/lit 2961 auth unification breaking remove existing authsigs

Open Ansonhkg opened this issue 10 months ago • 2 comments

Description

  • Removed authSig as an option to pass in to endpoint functions eg. pkpSign executeJs and signSessionKey
  • Extracted out all parser, helper, transcompiler functions in the pkpSign and executeJs to a dedicated helpers folder with isolated unit tests
  • Removing getXXPromiseShares methods in the LitNodeClient & its interface, and replacing it with a generic generatePromise instead. See lit-node-client-nodejs.ts
  • Better types for IOs

Note

The commit history is a bit overlapping with other PRs because I need the tests and some other features, please review from the commit below onward (or I just open a new PR and cherry pick the commits if that's better, lmk.)

Type of change

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [x] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [x] This change requires a documentation update

How Has This Been Tested?

  • [x] Unit test (Jest)
  • [x] E2E test (Tinny)

Checklist:

  • [x] My code follows the style guidelines of this project
  • [ ] I have performed a self-review of my code
  • [x] I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [x] My changes generate no new warnings
  • [x] I have added tests that prove my fix is effective or that my feature works
  • [x] New and existing unit tests pass locally with my changes
  • [ ] Any dependent changes have been merged and published in downstream modules

Ansonhkg avatar Apr 25 '24 21:04 Ansonhkg

@Ansonhkg this looks awesome! Thinking to get ready for this merger we should make a release/6.0.0 branch where we can start merging this work into and doing test runs. WDYT? we then could start merging #444 #436

joshLong145 avatar Apr 29 '24 20:04 joshLong145

@Ansonhkg this looks awesome! Thinking to get ready for this merger we should make a release/6.0.0 branch where we can start merging this work into and doing test runs. WDYT? we then could start merging #444 #436

I think we should merge https://github.com/LIT-Protocol/js-sdk/pull/435 instead of these sub-branches

DashKash54 avatar Apr 29 '24 21:04 DashKash54

@Ansonhkg Still a bunch of unresolved comments. Can you please respond and resolve those, thanks

DashKash54 avatar May 01 '24 06:05 DashKash54