Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Skip API key with tx status endpoints #3849

Merged
merged 3 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions packages/bridge/src/skip/__tests__/skip-transfer-status.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ import { rest } from "msw";
import { MockChains } from "../../__tests__/mock-chains";
import { server } from "../../__tests__/msw";
import { BridgeEnvironment, TransferStatusReceiver } from "../../interface";
import { SkipTransferStatusProvider } from "../transfer-status";
import { SkipApiClient } from "../client";
import {
SkipStatusProvider,
SkipTransferStatusProvider,
} from "../transfer-status";

jest.mock("@osmosis-labs/utils", () => ({
...jest.requireActual("@osmosis-labs/utils"),
Expand All @@ -19,6 +23,14 @@ jest.mock("@osmosis-labs/utils", () => ({
}),
}));

const SkipStatusProvider: SkipStatusProvider = {
transactionStatus: ({ chainID, txHash, env }) => {
const client = new SkipApiClient(env);
return client.transactionStatus({ chainID, txHash });
},
trackTransaction: () => Promise.resolve(),
};

// silence console errors
jest.spyOn(console, "error").mockImplementation(() => {});

Expand All @@ -31,7 +43,8 @@ describe("SkipTransferStatusProvider", () => {
beforeEach(() => {
provider = new SkipTransferStatusProvider(
"mainnet" as BridgeEnvironment,
MockChains
MockChains,
SkipStatusProvider
);
provider.statusReceiverDelegate = mockReceiver;
});
Expand Down Expand Up @@ -108,7 +121,8 @@ describe("SkipTransferStatusProvider", () => {
it("should generate correct explorer URL for testnet", () => {
const testnetProvider = new SkipTransferStatusProvider(
"testnet" as BridgeEnvironment,
MockChains
MockChains,
SkipStatusProvider
);
const url = testnetProvider.makeExplorerUrl(
JSON.stringify({
Expand All @@ -123,7 +137,8 @@ describe("SkipTransferStatusProvider", () => {
it("should generate correct explorer URL for a cosmos chain", () => {
const cosmosProvider = new SkipTransferStatusProvider(
"mainnet" as BridgeEnvironment,
MockChains
MockChains,
SkipStatusProvider
);
const url = cosmosProvider.makeExplorerUrl(
JSON.stringify({
Expand Down
3 changes: 2 additions & 1 deletion packages/bridge/src/skip/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
export class SkipApiClient {
constructor(
readonly env: BridgeEnvironment,
protected readonly apiKey = process.env.SKIP_API_KEY,
readonly baseUrl = "https://api.skip.money"
) {}

Expand Down Expand Up @@ -122,7 +123,7 @@ export class SkipApiClient {
return apiClient<Response>(args[0], args[1]);
}

const key = process.env.SKIP_API_KEY;
const key = this.apiKey;
if (!key) throw new Error("SKIP_API_KEY is not set");

return apiClient<Response>(args[0], {
Expand Down
1 change: 1 addition & 0 deletions packages/bridge/src/skip/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -936,4 +936,5 @@ export class SkipBridgeProvider implements BridgeProvider {
}
}

export * from "./client";
export * from "./transfer-status";
83 changes: 49 additions & 34 deletions packages/bridge/src/skip/transfer-status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,23 @@ import type {
TransferStatusProvider,
TransferStatusReceiver,
} from "../interface";
import { SkipApiClient } from "./client";
import { SkipBridgeProvider } from "./index";
import { SkipTxStatusResponse } from "./types";

type Transaction = {
chainID: string;
txHash: string;
env: BridgeEnvironment;
};

export interface SkipStatusProvider {
transactionStatus: ({
chainID,
txHash,
env,
}: Transaction) => Promise<SkipTxStatusResponse>;
trackTransaction: ({ chainID, txHash, env }: Transaction) => Promise<void>;
}

/** Tracks (polls skip endpoint) and reports status updates on Skip bridge transfers. */
export class SkipTransferStatusProvider implements TransferStatusProvider {
Expand All @@ -19,12 +34,13 @@ export class SkipTransferStatusProvider implements TransferStatusProvider {

statusReceiverDelegate?: TransferStatusReceiver | undefined;

readonly skipClient: SkipApiClient;
readonly axelarScanBaseUrl: string;

constructor(env: BridgeEnvironment, protected readonly chainList: Chain[]) {
this.skipClient = new SkipApiClient(env);

constructor(
protected readonly env: BridgeEnvironment,
protected readonly chainList: Chain[],
protected readonly skipStatusProvider: SkipStatusProvider
) {
this.axelarScanBaseUrl =
env === "mainnet"
? "https://axelarscan.io"
Expand All @@ -40,39 +56,38 @@ export class SkipTransferStatusProvider implements TransferStatusProvider {

await poll({
fn: async () => {
try {
const txStatus = await this.skipClient.transactionStatus({
chainID: fromChainId.toString(),
txHash: sendTxHash,
const tx = {
chainID: fromChainId.toString(),
txHash: sendTxHash,
env: this.env,
};

const txStatus = await this.skipStatusProvider
.transactionStatus(tx)
.catch(async (error) => {
if (error instanceof Error && error.message.includes("not found")) {
// if we get an error that it's not found, prompt skip to track it first
// then try again
await this.skipStatusProvider.trackTransaction(tx);
return this.skipStatusProvider.transactionStatus(tx);
Copy link
Collaborator

@JoseRFelix JoseRFelix Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this call short-circuit the poll if it fails since this is returning a promise without a catch statement?

Copy link
Member Author

@jonator jonator Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYM without a catch statement? I believe what would happen here is it would be transformed into the same promise that is awaited at the initial call above.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wouldn’t, since this isn't a recursive call and it's using the dependency-injected method. If we want to share the catch statement, we’d likely need to create an inline function and call it inside the catch method

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow, how would this look?

Copy link
Collaborator

@JoseRFelix JoseRFelix Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I think we could avoid recursion by just adding a catch. It will just call the method on the next poll cycle

Suggested change
return this.skipStatusProvider.transactionStatus(tx);
return this.skipStatusProvider.transactionStatus(tx).catch(() => {});

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think recursion is being used, we're just calling the same given function twice

}

throw error;
});

let status: TransferStatus = "pending";
if (txStatus.state === "STATE_COMPLETED_SUCCESS") {
status = "success";
}

if (txStatus.state === "STATE_COMPLETED_ERROR") {
status = "failed";
}

return {
id: sendTxHash,
status,
};
} catch (error: any) {
if ("message" in error) {
if (error.message.includes("not found")) {
await this.skipClient.trackTransaction({
chainID: fromChainId.toString(),
txHash: sendTxHash,
});

return undefined;
}
}
let status: TransferStatus = "pending";
if (txStatus.state === "STATE_COMPLETED_SUCCESS") {
status = "success";
}

throw error;
if (txStatus.state === "STATE_COMPLETED_ERROR") {
status = "failed";
}

return {
id: sendTxHash,
status,
};
},
validate: (incomingStatus) => {
if (!incomingStatus) {
Expand Down
32 changes: 32 additions & 0 deletions packages/web/pages/api/skip-track-tx.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { BridgeEnvironment, SkipApiClient } from "@osmosis-labs/bridge";
import { NextApiRequest, NextApiResponse } from "next";

/** This edge function is necessary to invoke the SkipApiClient on the server
* as a secret API key is required for the client.
*/
export default async function skipTrackTx(
req: NextApiRequest,
res: NextApiResponse
) {
const { chainID, txHash, env } = req.query as {
chainID: string;
txHash: string;
env: BridgeEnvironment;
};

if (!chainID || !txHash || !env) {
return res.status(400).json({ error: "Missing required query parameters" });
}

const skipClient = new SkipApiClient(env);

try {
const status = await skipClient.trackTransaction({ chainID, txHash });
return res.status(200).json(status);
} catch (error) {
if (error instanceof Error) {
return res.status(500).json({ error: error.message });
}
return res.status(500).json({ error: "An unknown error occurred" });
}
}
32 changes: 32 additions & 0 deletions packages/web/pages/api/skip-tx-status.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { BridgeEnvironment, SkipApiClient } from "@osmosis-labs/bridge";
import { NextApiRequest, NextApiResponse } from "next";

/** This edge function is necessary to invoke the SkipApiClient on the server
* as a secret API key is required for the client.
*/
export default async function skipTxStatus(
req: NextApiRequest,
res: NextApiResponse
) {
const { chainID, txHash, env } = req.query as {
chainID: string;
txHash: string;
env: BridgeEnvironment;
};

if (!chainID || !txHash || !env) {
return res.status(400).json({ error: "Missing required query parameters" });
}

const skipClient = new SkipApiClient(env);

try {
const status = await skipClient.transactionStatus({ chainID, txHash });
return res.status(200).json(status);
} catch (error) {
if (error instanceof Error) {
return res.status(500).json({ error: error.message });
}
return res.status(500).json({ error: "An unknown error occurred" });
}
}
28 changes: 27 additions & 1 deletion packages/web/stores/root.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,33 @@ export class RootStore {
),
new SkipTransferStatusProvider(
IS_TESTNET ? "testnet" : "mainnet",
ChainList
ChainList,
{
transactionStatus: async ({ chainID, txHash, env }) => {
const response = await fetch(
`/api/skip-tx-status?chainID=${chainID}&txHash=${txHash}&env=${env}`
);
const responseJson = await response.json();
if (!response.ok) {
throw new Error(
"Failed to fetch transaction status: " + responseJson.error
);
}
return responseJson;
},
trackTransaction: async ({ chainID, txHash, env }) => {
const response = await fetch(
`/api/skip-track-tx?chainID=${chainID}&txHash=${txHash}&env=${env}`
);
const responseJson = await response.json();
if (!response.ok) {
throw new Error(
"Failed to track transaction: " + responseJson.error
);
}
return responseJson;
},
jonator marked this conversation as resolved.
Show resolved Hide resolved
}
jonator marked this conversation as resolved.
Show resolved Hide resolved
),
new IbcTransferStatusProvider(ChainList, AssetLists),
];
Expand Down
Loading