Skip to content

Commit ca0df6a

Browse files
committedJul 17, 2024
sdk in design
1 parent 4063d0b commit ca0df6a

File tree

2 files changed

+31
-9
lines changed

2 files changed

+31
-9
lines changed
 

‎DESIGN.md

+17-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1+
# Design
12

2-
# Structure
3+
## Structure
34

45
A primary goal for the SDK was to provide modular and specific access to the protocols build on Wormhole.
56

@@ -30,7 +31,9 @@ sdk/ -- Metapackage that depeneds on the rest of the packages, meant to be a sim
3031
examples/ -- Examples, also used for README string replace
3132
```
3233

33-
# Platform Interfaces
34+
# Interfaces
35+
36+
## Platform Interfaces
3437

3538
Several interfaces should be available for each platform supported.
3639

@@ -39,8 +42,19 @@ Several interfaces should be available for each platform supported.
3942
- A `Signer` is an interface that provides a callback to `sign` or `signAndSend` one or more transaction objects.
4043
- An `Address` provides parsing/formatting/conversion for platform specific addresses.
4144

42-
# Protocol Interfaces
45+
## Protocol Interfaces
4346

4447
A Protocol (fka `Module`) is a specific application, it provides a set of methods that can be called to accomplish some action (e.g. `TokenBridge` allows `transfer`/`redeem`/`getWrappedAsset`, etc...)
4548

4649
To allow platform agnostic access to Protocols, each Platform that provides the protocol should have its own implementation.
50+
51+
# SDK package
52+
53+
The `@wormhole-foundation/sdk` package was created to reduce the confusion reported by devs around what they needed to install and use.
54+
55+
Because this project has strict separation of platform implementation packages, the registration of platforms and protocols is done as a side effect. As a result, naked imports may be required for lower level packages to be properly registered.
56+
57+
This package specifies _all_ platforms and protocols as dependencies which increases installed size but provides provides conditional exports for each `Platform` to allow for a smaller bundle size.
58+
59+
Each platform has a `PlatformDefiniton` containing the `Platform` specific implementations that can be imported directly or through the `PlatformLoader` which ensures the protocols are registered as well.
60+

‎notes.md

+14-6
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,19 @@ Examples:
2929

3030
The [tokens](./core/base/src/constants/tokens) consts take up a lot of packed space for marginal benefit. Consider making these a wholly separate package that _can_ be installed and fetched when necessary.
3131

32-
The [nativeChainIds](./core/base/src/constants/nativeChainIds.ts) constants mix different chain id types, causing extra akward type checks to be done. Consider splitting these by platform so that when a `Chain` is passed, the type of chain id can be inferred. Also will help to distinguish between chain id _kinds_ (eip155 vs whatever).
32+
The [nativeChainIds](./core/base/src/constants/nativeChainIds.ts) constants mix different chain id types, causing extra akward type checks to be done. Consider splitting these by platform so that when a `Chain` is passed, the type of chain id can be inferred. Also will help to distinguish between chain id _kinds_ (eip155 vs whatever) since they should be unique at least in that context.
3333

3434
### Definitions
3535

3636
The [layouts](./core/definitions/src/protocols/tokenBridge/tokenBridgeLayout.ts) for the protocols are very difficult to read/comprehend from just source. We should have some way to write out the full object produced so that the fields it contains are easier to read.
3737

3838
The [Attestation/AttestationId](./core/definitions/src/attestation.ts) should be registered along with the protocol instead of having a hardcoded switch case. The current definition makes it very awkward to use for anything outside this repo, but since its used in the Routes, external "plugins" need to have _something_. This forces the Attestation type to be a punted `VAA<"Uint8Array">`
3939

40-
The [RPC](./core/definitions/src/rpc.ts) is a punted `any` type. It could benefit from the same type registration as others, where the exact type is set in some namespace so that type inteference would be happy with something like `(await solChain.getRpc()) satisfies Connection`
40+
The [RPC](./core/definitions/src/rpc.ts) is a punted `any` type. It could benefit from the same type registration as others, where the exact type is set in some namespace so that type inteference would be happy with something like:
41+
42+
```ts
43+
(await ChainContext<"Mainnet", "Solana">.getRpc()) satisfies Connection
44+
```
4145

4246
The [protocols](./core/definitions/src/protocols/) should have *standard* fields in their respective `namespace` for things like:
4347

@@ -51,19 +55,19 @@ The [protocols](./core/definitions/src/protocols/) should have *standard* fields
5155
## Connect
5256

5357

54-
The [Wormhole](./connect/src/wormhole.ts) class has spotty coverage of util methods. Some of the methods are awkward to have on `Wormhole` like `canonicalAddress` which is already an exported method.
58+
The [Wormhole](./connect/src/wormhole.ts) class has spotty coverage of util methods. Some of the methods are awkward to have on `Wormhole` like `canonicalAddress` which is already an exported method.
5559

5660
The [Receipt typeguards](./connect/src/types.ts) should also define `hasSourceInitiated`, `hasAttested`, etc for checking if the transfer is _past_ some step.
5761

5862
The [Wormholescan API](./connect/src/whscan-api.ts) should be better documented and possibly autogenerated from the swagger for the api.
5963

60-
The classes that implement the [WormholeTransfer](./connect/src/protocols/wormholeTransfer.ts) should be deprecated in favor of the `Route` usage.
64+
The classes that implement the [WormholeTransfer](./connect/src/protocols/wormholeTransfer.ts) should be deprecated in favor of the `Route` usage. The underlying code should be preserved and made available in a static context where all args are provided rather than keeping state in the `WormholeTransfer` object.
6165

6266
## Platforms
6367

6468
Every Protocol implementation defines its own private `createUnsignedTransaction` function, which, kinda sucks.
6569

66-
The Signer implementations are bad at things like gas estimation or handling errors. It should also provide better support for transaction review prior to signing and sending.
70+
The Signer implementations are bad at things like gas estimation or handling errors. It should also provide support for transaction review prior to signing.
6771

6872

6973
### Evm
@@ -76,7 +80,7 @@ Consider removing [Portico](./platforms/evm/protocols/portico/) completely from
7680

7781
A lot of the code here was copy/pasted without regard to what was actually necessary. Consider purging any unused functions.
7882

79-
While the `Buffer` type has been kept out of most packages, the Solana packages seem to really want it for now. When the next version of the Solana SDK is released, consider trying to purge it completely.
83+
While the `Buffer` type has been kept out of most packages intentionally, the Solana packages have not been as strict. For web applications this can still be a pain, consider trying to purge it completely.
8084

8185
The [Unsigned Transaction](./platforms/solana/src/unsignedTransaction.ts) type can handle versioned transactions but we still only produce the legacy transaction type. We should produce the versioned transactions since they'll provide more features in the future, [issue here](https://github.com/wormhole-foundation/wormhole-sdk-ts/issues/163).
8286

@@ -88,3 +92,7 @@ The IBC channel consts need to be updated with the [fetch-registry](./platforms/
8892

8993
The IBC protocol implementation should probably have just been `Gateway` instead or a new Protocol for `Gateway` specific methods should have been created. The connect package GatewayTransfer is made awkard by the current setup.
9094

95+
### Sui
96+
97+
Disaster show wrt types, a lot of weird type checking to get some deeply nested field in a MoveValue.
98+

0 commit comments

Comments
 (0)