ZK-559: Remove "/crypto" package from shielder-sdk #81
Conversation
|
| @@ -46,18 +45,12 @@ jobs: | |||
| uses: ./.github/workflows/_ts-checks.yml | |||
| secrets: inherit | |||
|
|
|||
| ts-sdk-playwright-tests: | |||
There was a problem hiding this comment.
Why are we removing the workflow entirely? Playwright is needed for testing the smaller shielder-sdk-crypto-wasm package.
There was a problem hiding this comment.
we can add it later
ts/shielder-sdk/README.md
Outdated
|
|
||
| ## Usage | ||
|
|
||
| In order to use most of the modules, WASM needs to be loaded. This package starts loading WASM automatically. User should await the published promise `wasmClientWorkerReady()`. | ||
| The main entry point is the `createShielderClient` function which provides methods for interacting with the Shielder protocol. |
There was a problem hiding this comment.
Forgive my focusing on a tiny language nitpick, but please add a comma before "which":P Not having a comma means there is many createShielderClient functions and we're choosing the one which provides methods for interacting with the Shielder protocol.
| @@ -119,7 +85,6 @@ jobs: | |||
| NO_FORMATTING=true TESTNET=true ./tooling-e2e-tests/full_scenario.sh | |||
| NO_FORMATTING=true TESTNET=true ./tooling-e2e-tests/recovery_scenario.sh | |||
| NO_FORMATTING=true TESTNET=true ./tooling-e2e-tests/many_actors.sh | |||
| NO_FORMATTING=true TESTNET=true ./tooling-e2e-tests/ts_sdk_tests.sh | |||
There was a problem hiding this comment.
Consider removing shielder-sdk-tests code entirely. We have repo history, not having dead code seems preferrable.
|
|
||
| // Initialize the client | ||
| const shielderClient = createShielderClient( |
There was a problem hiding this comment.
Consider replacing fake variables with literal fake values like 0xcccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc. This would improve readability as the reader then sees the formatting of the expected arguments.
| }; | ||
| } | ||
|
|
||
| async merklePathAndRoot( |
There was a problem hiding this comment.
Could you move this helper out of NoteAction? An isolated stateless helper is simpler than keeping it inside an abstract base class. It seems unnecessary to require the user to create an object like DepositAction for a Merkle path query. The tests will have to create the Merkle path, it seems simpler to have this call decoupled.
This PR introduces changes to
shielder-sdk, effectively making it independent of native cryptography implementation../src/cryptopackage and all configuration of WASM, includingshielder-wasmbindings.shielder-sdk-cryptointerfaceCryptoClientinterface for calling cryptographic functions where needed.shielder-sdk-testsa dead code, as we intend to rewrite them into simpler framework soon. Disabling CI forshielder-sdk-testspackage