[PSDK-187] Server Signer Support #42

John-peterson-coinbase · 2024-05-31T15:33:11Z

What changed? Why?

Adds support for server signers.
Adds CoinbaseOptions and CoinbaseConfigureFromJsonOptions types to pass to SDK initializers

Qualified Impact

jazz-cb · 2024-06-03T14:13:42Z

README.md

 ```

 Another way to initialize the SDK is by sourcing the API key from the json file that contains your API key, downloaded from CDP portal.

 ```typescript
-const coinbase = Coinbase.configureFromJson("path/to/your/api-key.json");
+const coinbase = Coinbase.configureFromJson({ filePath: "path/to/your/api-key.json" });


we should fix this in docs as well if this is the case @erdimaden

agreed, we need to update the docs.

+1 docs need to be updated to use new paradigm

@John-peterson-coinbase let's use privateKey instead of apiKeyPrivateKey so we can just use

Suggested change

const coinbase = Coinbase.configureFromJson({ filePath: "path/to/your/api-key.json" });

const coinbase = new Coinbase({ apiKeyName, privateKey });

jazz-cb · 2024-06-03T14:45:10Z

src/coinbase/coinbase.ts

@@ -52,23 +51,28 @@ export class Coinbase {
   */
  static apiKeyPrivateKey: string;

+  /**
+   * Whether to use server signer or not.


nit: whether to use a server signer to manage private keys.

erdimaden · 2024-06-03T14:59:39Z

src/coinbase/address.ts

-    const broadcastTransferRequest = {
-      signed_payload: signedPayload,
-    };
+    if (!Coinbase.useServerSigner) {


I think we just need to return the transfer here if(Coinbase.useServerSigner)

https://github.com/coinbase/coinbase-sdk-ruby/blob/v0.0.6/lib/coinbase/address.rb#L89

We use native async/await primitives on node.js side so we cannot just return the transfer.

erdimaden · 2024-06-03T15:01:02Z

src/coinbase/address.ts


    const startTime = Date.now();
    while (Date.now() - startTime < timeoutSeconds * 1000) {
-      const status = await updatedTransfer.getStatus();
+      await transfer.reload();
+      const status = transfer.getStatus();


do we need to call transfer.reload() for if(Coinbase.useServerSigner)

Yes because that is how we update the state of the transfer

erdimaden · 2024-06-03T15:02:42Z

src/coinbase/coinbase.ts

+  constructor(options: CoinbaseOptions) {
+    const apiKeyName = options.apiKeyName ?? "";
+    const privateKey = options.privateKey ?? "";
+    const useServerSigner = options.useServerSigner === true;
+    const debugging = options.debugging === true;
+    const basePath = options.basePath ?? BASE_PATH;


Suggested change

constructor(options: CoinbaseOptions) {

const apiKeyName = options.apiKeyName ?? "";

const privateKey = options.privateKey ?? "";

const useServerSigner = options.useServerSigner === true;

const debugging = options.debugging === true;

const basePath = options.basePath ?? BASE_PATH;

constructor({apiKeyName, privateKey, useServerSigner, useServerSigner, debugging = true, basePath= BASE_PATH}: CoinbaseOptions) {

erdimaden · 2024-06-03T15:03:44Z

src/coinbase/coinbase.ts

-    debugging: boolean = false,
-    basePath: string = BASE_PATH,
-  ): Coinbase {
+  static configureFromJson(options: CoinbaseConfigureFromJsonOptions): Coinbase {


the same object destructuring can be here

erdimaden · 2024-06-03T15:06:59Z

src/coinbase/wallet.ts

+    const startTime = Date.now();
+    while (Date.now() - startTime < timeoutSeconds * 1000) {
+      const response = await Coinbase.apiClients.wallet!.getWallet(walletId);
+      if (response?.data.server_signer_status === ServerSignerStatus.ACTIVE) {
+        return;
+      }
+      await delay(intervalSeconds);
+    }


can we have a generic util function for this logic? we use the exactly same in address class.

Yes lets follow up with this change since it'll add scope outside of server signer support to change in transfers etc

+1 to splitting out the logic however

erdimaden · 2024-06-03T15:08:09Z

README.md

 ```

 Another way to initialize the SDK is by sourcing the API key from the json file that contains your API key, downloaded from CDP portal.

 ```typescript
-const coinbase = Coinbase.configureFromJson("path/to/your/api-key.json");
+const coinbase = Coinbase.configureFromJson({ filePath: "path/to/your/api-key.json" });


agreed, we need to update the docs.

John-peterson-coinbase force-pushed the feat/serversigner branch from 164ea2e to 673afc8 Compare May 31, 2024 15:34

John-peterson-coinbase added 3 commits June 2, 2024 19:59

[PSDK-187] Server Signer Support

b3a8943

tests

4abe5d9

use configuration option types for constructors

b3fa9b2

John-peterson-coinbase force-pushed the feat/serversigner branch from 31ef3d7 to f118dc4 Compare June 3, 2024 03:04

typedocs

827eb68

John-peterson-coinbase force-pushed the feat/serversigner branch from f118dc4 to 827eb68 Compare June 3, 2024 03:06

John-peterson-coinbase changed the title ~~🚧[WIP] [PSDK-187] Server Signer Support 🚧~~ [PSDK-187] Server Signer Support Jun 3, 2024

John-peterson-coinbase marked this pull request as ready for review June 3, 2024 03:09

nits

510c573

John-peterson-coinbase requested a review from jazz-cb June 3, 2024 03:19

readme

173b128

jazz-cb requested review from erdimaden and alex-stone June 3, 2024 14:12

jazz-cb reviewed Jun 3, 2024

View reviewed changes

erdimaden reviewed Jun 3, 2024

View reviewed changes

John-peterson-coinbase added 3 commits June 3, 2024 08:30

pr review

7f681e1

update docs

6ebd434

update typedocs

4f9f71c

John-peterson-coinbase requested review from erdimaden and jazz-cb June 3, 2024 15:38

jazz-cb approved these changes Jun 3, 2024

View reviewed changes

John-peterson-coinbase merged commit 048c586 into v0.0.6 Jun 3, 2024
3 checks passed

erdimaden deleted the feat/serversigner branch June 21, 2024 18:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PSDK-187] Server Signer Support #42

[PSDK-187] Server Signer Support #42

John-peterson-coinbase commented May 31, 2024 •

edited

Loading

jazz-cb Jun 3, 2024

erdimaden Jun 3, 2024

John-peterson-coinbase Jun 3, 2024

erdimaden Jun 3, 2024

jazz-cb Jun 3, 2024

erdimaden Jun 3, 2024

John-peterson-coinbase Jun 3, 2024

erdimaden Jun 3, 2024

John-peterson-coinbase Jun 3, 2024

erdimaden Jun 3, 2024

erdimaden Jun 3, 2024

erdimaden Jun 3, 2024

John-peterson-coinbase Jun 3, 2024

John-peterson-coinbase Jun 3, 2024

erdimaden Jun 3, 2024

	const coinbase = Coinbase.configureFromJson({ filePath: "path/to/your/api-key.json" });
	const coinbase = new Coinbase({ apiKeyName, privateKey });

[PSDK-187] Server Signer Support #42

[PSDK-187] Server Signer Support #42

Conversation

John-peterson-coinbase commented May 31, 2024 • edited Loading

What changed? Why?

Qualified Impact

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

John-peterson-coinbase commented May 31, 2024 •

edited

Loading