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

Swap failed events: only reject committed txs with code !== 0 #3301

Merged
merged 4 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
8 changes: 2 additions & 6 deletions packages/web/components/swap-tool/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,8 @@ export const SwapTool: FunctionComponent<SwapToolProps> = observer(
});
}
})
.catch((error) => {
console.error("swap failed", error);
if (error instanceof Error && error.message === "Request rejected") {
// don't log when the user rejects in wallet
return;
}
.catch(() => {
console.log("reject sig");
logEvent([EventName.Swap.swapFailed, baseEvent]);
})
.finally(() => {
Expand Down
46 changes: 26 additions & 20 deletions packages/web/hooks/use-swap.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -269,14 +269,14 @@ export function useSwap(
() =>
new Promise<"multiroute" | "multihop" | "exact-in">(
async (resolve, reject) => {
if (!maxSlippage) return reject("No max slippage");
if (!inAmountInput.amount) return reject("No input amount");
if (!account) return reject("No account");
if (!swapAssets.fromAsset) return reject("No from asset");
if (!swapAssets.toAsset) return reject("No to asset");
if (!maxSlippage) return reject(new Error("No max slippage"));
if (!inAmountInput.amount)
return reject(new Error("No input amount"));
if (!account) return reject(new Error("No account"));
if (!swapAssets.fromAsset) return reject(new Error("No from asset"));
if (!swapAssets.toAsset) return reject(new Error("No to asset"));
jonator marked this conversation as resolved.
Show resolved Hide resolved

let txParams: ReturnType<typeof getSwapTxParameters>;

try {
txParams = getSwapTxParameters({
coinAmount: inAmountInput.amount.toCoin().amount,
Expand All @@ -287,7 +287,7 @@ export function useSwap(
});
} catch (e) {
const error = e as Error;
return reject(error.message);
return reject(error);
jonator marked this conversation as resolved.
Show resolved Hide resolved
}

const { routes, tokenIn, tokenOutMinAmount } = txParams;
Expand Down Expand Up @@ -344,7 +344,7 @@ export function useSwap(
);
});
} catch (e) {
return reject("Rejected manual approval");
return reject(new Error("Rejected manual approval"));
jonator marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -373,14 +373,15 @@ export function useSwap(
tokenOutMinAmount,
undefined,
signOptions,
() => {
resolve(pools.length === 1 ? "exact-in" : "multihop");
({ code }) => {
if (code)
reject(
new Error("Failed to send swap exact amount in message")
);
else resolve(pools.length === 1 ? "exact-in" : "multihop");
Comment on lines +382 to +387
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor error handling to include error codes for better diagnostics.

- reject(new Error("Failed to send swap exact amount in message"));
+ reject(new Error(`Failed to send swap exact amount in message with error code: ${code}`));
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
({ code }) => {
if (code)
reject(
new Error("Failed to send swap exact amount in message")
);
else resolve(pools.length === 1 ? "exact-in" : "multihop");
({ code }) => {
if (code)
reject(
new Error(`Failed to send swap exact amount in message with error code: ${code}`)
);
else resolve(pools.length === 1 ? "exact-in" : "multihop");

}
)
.catch((reason) => {
reject(reason);
});
return pools.length === 1 ? "exact-in" : "multihop";
.catch(reject);
} else if (routes.length > 1) {
account.osmosis
.sendSplitRouteSwapExactAmountInMsg(
Expand All @@ -389,15 +390,20 @@ export function useSwap(
tokenOutMinAmount,
undefined,
signOptions,
() => {
resolve("multiroute");
({ code }) => {
if (code)
reject(
new Error(
"Failed to send split route swap exact amount in message"
)
);
jonator marked this conversation as resolved.
Show resolved Hide resolved
else resolve("multiroute");
}
)
.catch((reason) => {
reject(reason);
});
.catch(reject);
} else {
reject("No routes given");
// should not be possible because button should be disabled
jonator marked this conversation as resolved.
Show resolved Hide resolved
reject(new Error("No routes given"));
jonator marked this conversation as resolved.
Show resolved Hide resolved
}
}
).finally(() => {
Expand Down
Loading