feat(per-js-sdk): Add support for bid state and raw bid submission #1329

m30m · 2024-02-29T12:31:42Z

No description provided.

vercel · 2024-02-29T12:31:44Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments

Name	Status	Preview	Comments	Updated (UTC)
example-oracle-amm	⬜️ Ignored (Inspect)	Visit Preview		Mar 14, 2024 7:08pm
xc-admin-frontend	⬜️ Ignored (Inspect)	Visit Preview		Mar 14, 2024 7:08pm

anihamde · 2024-02-29T15:09:04Z

express_relay/sdk/js/src/examples/SimpleSearcher.ts

    throw new Error(`Invalid private key: ${argv.privateKey}`);
  }
  const DAY_IN_SECONDS = 60 * 60 * 24;
+  client.setBidStatusHandler(async (bidStatus) => {


i think we should define these things within a simple searcher class, just makes it easier to find and read then an inline function

anihamde · 2024-02-29T15:12:13Z

express_relay/sdk/js/src/index.ts

+        if (this.websocketBidStatusCallback !== undefined) {
+          await this.websocketBidStatusCallback(message);
+        }
+      } else if ("id" in message && message.id) {


can you add some comments here for what this path is supposed to be?

anihamde · 2024-02-29T15:19:58Z

express_relay/sdk/js/src/index.ts

    });
  }

+  async submitOpportunityBidViaWebsocket(bid: OpportunityBid): Promise<BidId> {


i think there's probably better clearer naming here, but that probably depends on us developing clearer terminology for these different parts of the server. rn the terms OpportunityBid vs Bid are not very clear at first inspection, maybe something like submitLiquidationAdapterBid vs. submitBid?

I agree that our terminology is a bit confusing but not sure about your suggestion. the term adapter has never been exposed in the api so I think it makes it more confusing here. I guess I have made a decision early on the summarise the LiquidationOpportunity term by dropping the liquidation part. LiquidationOpportunityBid is more verbose but also a bit long. I'm open to suggestions but terminology changes should be consistent across different docs and services.

I think this depends on what we call the LiquidationAdapter server to differentiate it from the auction server. maybe we ought to refine that naming distinction, and that will make it clearer: for example, naming it "Liquidation Forum"?

exposing the viaWebsocket to users seems like it's exposing an implementation detail of how this works. There's a functional difference between this and the post method below though, which is that you get updates on the bid if you use websocket.

I think a better way to expose this functionality would be to have one method like submitOpportunityBid that has a default parameter like subscribeToUpdates = true, and then switches between websocket and post depending on that.

anihamde · 2024-02-29T15:22:13Z

express_relay/sdk/js/src/index.ts

+/**
+ * Represents a raw bid on acquiring a permission key
+ */
+export type Bid = {


this type is in the generated types. we should use that and maybe perform some validation atop that

this is not quite the same type unfortunately. there is a snake_case -> camelCase conversion and I chose types that made more sense in a js sdk instead of the general openapi types. I'm not sure if there is an easy way to apply these changes on top of the openapi type.

anihamde · 2024-02-29T15:34:00Z

express_relay/sdk/js/src/index.ts

+    bid: OpportunityBid
+  ): components["schemas"]["OpportunityBid"] {
+    return {
+      amount: bid.bid.amount.toString(),


does the type generation have a way to auto convert to json format? or do like json.stringify or something? then you don't have to rewrite this whenever the type changes in versions

They just suggest using snake_case everywhere but it's a bit too late for that. I guess anything magical here would remove the current type safety checks we have. Do you have anything in mind?

nit: I think if the type is identical to server type you can consider using https://quicktype.io/ to generate types with conversion based on the JSON Schema of the API.

ali-behjati · 2024-03-01T11:42:18Z

express_relay/sdk/js/src/index.ts

+  }
+
+  async submitBidViaWebsocket(bid: Bid): Promise<BidId> {
+    const result = await this.sendWebsocketMessage({


nit nit: i think sendWebsocketMessage can be renamed to something that indicates there is a response but I can't think of a good name myself.

ali-behjati · 2024-03-01T11:46:04Z

express_relay/sdk/js/src/index.ts

+    bid: OpportunityBid
+  ): components["schemas"]["OpportunityBid"] {
+    return {
+      amount: bid.bid.amount.toString(),


nit: I think if the type is identical to server type you can consider using https://quicktype.io/ to generate types with conversion based on the JSON Schema of the API.

ali-behjati

Nice!
I left some minor inline comments.

Looking at the sdk I also have a mixed feeling about having two ways of doing things. This can also appear confusing to people (and the trade-offs are not very clear). Do we still need to maintain HTTP apis. If so, I recommend documenting the trade-offs.

ali-behjati · 2024-03-01T11:49:06Z

express_relay/sdk/js/src/examples/SimpleSearcher.ts

+    this.client.setBidStatusHandler(this.bidStatusHandler.bind(this));
+    this.client.setOpportunityHandler(this.opportunityHandler.bind(this));


I still think that this is not a non-intutive API and recommend moving it to the constructor of Client.

anihamde · 2024-03-14T16:44:40Z

express_relay/sdk/js/src/examples/SimpleSearcher.ts

+  }
+
+  async bidStatusHandler(bidStatus: BidStatusUpdate) {
+    console.log(`Bid status for bid ${bidStatus.id}: ${bidStatus.status}`);


log the result when the status is submitted

anihamde · 2024-03-14T16:48:35Z

express_relay/sdk/js/src/index.ts

 */
-export type TokenQty = {
-  contract: Address;
+export type TokenAmount = {


let's move these types into a different file

anihamde · 2024-03-14T16:49:11Z

express_relay/sdk/js/src/index.ts

- * All the parameters necessary to represent a liquidation opportunity
+ * All the parameters necessary to represent an opportunity
 */
 export type Opportunity = {


make a separate OpportunityParams struct for supporting surfacing opportunities

anihamde · 2024-03-14T16:52:58Z

express_relay/sdk/js/src/examples/SimpleSearcher.ts

+  }
+
+  async opportunityHandler(opportunity: Opportunity) {
+    const bid = BigInt(argv.bid);


this doesn't need to be a cl arg, it's just a constant for the purposes of illustration in this script

this was useful internally as a dummy test to make sure auction server ignores lower bids. I'd run two scripts with different bid params and see who wins. I removed it from the docs, but will leave it here since there is some default value and it will not complicate the users testing flow.

feat(per-js-sdk): Add support for bid state and raw bid submission

3062c25

Add custom error class for easier excpetion handling

d87ac1a

anihamde reviewed Feb 29, 2024

View reviewed changes

Address comments

78073d4

ali-behjati approved these changes Mar 1, 2024

View reviewed changes

m30m added 5 commits March 6, 2024 18:21

Address more comments

8f2535c

Merge websocket/http apis inside a single function

2933291

Rename bidInfo->bidParams

055ecf3

Adapt to new names on the server

b1ad2d3

Adapt field names

0fcbe37

anihamde approved these changes Mar 14, 2024

View reviewed changes

Address comments

a5b2972

m30m merged commit f14dd35 into main Mar 14, 2024

m30m deleted the per-js-sdk-updates branch March 14, 2024 19:15

		this.client.setBidStatusHandler(this.bidStatusHandler.bind(this));
		this.client.setOpportunityHandler(this.opportunityHandler.bind(this));

feat(per-js-sdk): Add support for bid state and raw bid submission #1329

feat(per-js-sdk): Add support for bid state and raw bid submission #1329

Uh oh!

Conversation

m30m commented Feb 29, 2024

Uh oh!

vercel bot commented Feb 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ali-behjati Mar 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ali-behjati left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vercel bot commented Feb 29, 2024 •

edited

Loading

ali-behjati Mar 1, 2024 •

edited

Loading