-
Notifications
You must be signed in to change notification settings - Fork 415
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
(Deposit/Withdrawal) Fetch Bridge Provider Quote #3402
Changes from all commits
4f805ca
252e0d0
b0f1c8a
9b462f8
681c4e3
c1136b0
2947d22
39124f2
2b8924c
f653001
9135ad1
566e72a
1162d59
84e92fa
2e8524e
1e60d72
bb6245b
acdccec
11e6be5
27f7677
63570fa
c1ceb71
f29a97b
2868eba
8be9aa4
3d5bb06
b838ad5
0fdeb42
8981144
fb38d8e
42873d2
3564c49
3a42b4b
9e9541a
ce88df4
0b80ced
ad185a5
55007b6
28192b8
1f01db5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,11 @@ | ||
import { SourceChain } from "../chain"; | ||
import { EthereumChainInfo } from "../ethereum"; | ||
import { AxelarSourceChain, EthereumChainInfo } from "@osmosis-labs/utils"; | ||
|
||
import { BridgeEnvironment } from "../interface"; | ||
|
||
/** @deprecated Prefer using Axelar chain/asset list API via bridge providers instead */ | ||
export type SourceChainTokenConfig = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's add @deprecated here as we don't need it after we commit to new d/w flow I thought I did this in a prev PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see that it has been added after merging with stage |
||
/** Source Chain identifier. */ | ||
id: SourceChain; | ||
id: AxelarSourceChain; | ||
chainId?: number; | ||
/** Address of origin ERC20 token for that origin chain. Leave blank to | ||
* prefer native ETH currency if `id` is not a Cosmos chain in `ChainInfo`. | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,40 @@ | ||||||||||||||
import { | ||||||||||||||
EthereumChainInfo, | ||||||||||||||
NativeEVMTokenConstantAddress, | ||||||||||||||
} from "@osmosis-labs/utils"; | ||||||||||||||
import { Address, createPublicClient, erc20Abi, http } from "viem"; | ||||||||||||||
|
||||||||||||||
export async function getEvmBalance({ | ||||||||||||||
address, | ||||||||||||||
userAddress, | ||||||||||||||
chainId, | ||||||||||||||
}: { | ||||||||||||||
address: string; | ||||||||||||||
userAddress: string; | ||||||||||||||
chainId: number; | ||||||||||||||
}) { | ||||||||||||||
Comment on lines
+7
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve type safety for function parameters. The function parameters can be improved by defining a TypeScript interface for the input object. interface GetEvmBalanceParams {
address: string;
userAddress: string;
chainId: number;
}
export async function getEvmBalance({
address,
userAddress,
chainId,
}: GetEvmBalanceParams) {
// function body
} |
||||||||||||||
const evmChain = Object.values(EthereumChainInfo).find( | ||||||||||||||
(chain) => String(chain.id) === String(chainId) | ||||||||||||||
); | ||||||||||||||
|
||||||||||||||
if (!evmChain) { | ||||||||||||||
throw new Error(`Chain with id ${chainId} not found`); | ||||||||||||||
} | ||||||||||||||
Comment on lines
+20
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure consistent error messages. The error message should include more context to help with debugging. - throw new Error(`Chain with id ${chainId} not found`);
+ throw new Error(`Ethereum chain with id ${chainId} not found in EthereumChainInfo`); Committable suggestion
Suggested change
|
||||||||||||||
|
||||||||||||||
const publicClient = createPublicClient({ | ||||||||||||||
chain: evmChain, | ||||||||||||||
transport: http(evmChain.rpcUrls.default.http[0]), | ||||||||||||||
}); | ||||||||||||||
|
||||||||||||||
const balance = | ||||||||||||||
address === NativeEVMTokenConstantAddress | ||||||||||||||
? await publicClient.getBalance({ address: userAddress as Address }) | ||||||||||||||
: await publicClient.readContract({ | ||||||||||||||
abi: erc20Abi, | ||||||||||||||
address: address as Address, | ||||||||||||||
functionName: "balanceOf", | ||||||||||||||
args: [userAddress as Address], | ||||||||||||||
}); | ||||||||||||||
Comment on lines
+29
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle potential errors in contract calls. Ensure that potential errors in contract calls are handled gracefully. let balance;
try {
balance =
address === NativeEVMTokenConstantAddress
? await publicClient.getBalance({ address: userAddress as Address })
: await publicClient.readContract({
abi: erc20Abi,
address: address as Address,
functionName: "balanceOf",
args: [userAddress as Address],
});
} catch (error) {
console.error(`Failed to fetch balance for address ${address}: ${error.message}`);
throw error;
} |
||||||||||||||
|
||||||||||||||
return balance; | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure balance is returned in a consistent format. Consider ensuring that the returned balance is in a consistent format (e.g., a string or a specific unit) to avoid potential issues in downstream code. - return balance;
+ return balance.toString(); Committable suggestion
Suggested change
|
||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -67,7 +67,7 @@ export function getAssetWithVariants({ | |||||
return ( | ||||||
variants | ||||||
// place the canonical asset at the beginning | ||||||
.sort((a) => (a.coinDenom === asset.variantGroupKey ? -1 : 1)) | ||||||
.sort((a) => (a.coinMinimalDenom === asset.variantGroupKey ? -1 : 1)) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Omit the redundant else clause in the sort operation. The else clause is redundant because the previous branch breaks early. - .sort((a) => (a.coinMinimalDenom === asset.variantGroupKey ? -1 : 1))
+ .sort((a) => a.coinMinimalDenom === asset.variantGroupKey ? -1 : 1) Committable suggestion
Suggested change
|
||||||
); | ||||||
} | ||||||
|
||||||
|
@@ -193,6 +193,7 @@ function filterAssetList( | |||||
export * from "./bridge"; | ||||||
export * from "./categories"; | ||||||
export * from "./config"; | ||||||
export * from "./ethereum"; | ||||||
export * from "./gas"; | ||||||
export * from "./market"; | ||||||
export * from "./price"; | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,20 +31,20 @@ export async function getAssetWithUserBalance<TAsset extends MinimalAsset>({ | |
assetLists, | ||
chainList, | ||
asset, | ||
userOsmoAddress, | ||
userCosmosAddress, | ||
}: { | ||
assetLists: AssetList[]; | ||
chainList: Chain[]; | ||
asset: TAsset; | ||
userOsmoAddress?: string; | ||
userCosmosAddress?: string; | ||
}): Promise<TAsset & MaybeUserAssetCoin> { | ||
if (!userOsmoAddress) return asset; | ||
if (!userCosmosAddress) return asset; | ||
|
||
const userAssets = await mapGetAssetsWithUserBalances({ | ||
assetLists, | ||
chainList, | ||
assets: [asset], | ||
userOsmoAddress, | ||
userCosmosAddress: userCosmosAddress, | ||
includePreview: true, | ||
}); | ||
return userAssets[0]; | ||
|
@@ -62,14 +62,14 @@ export async function mapGetAssetsWithUserBalances< | |
assetLists: AssetList[]; | ||
chainList: Chain[]; | ||
assets?: TAsset[]; | ||
userOsmoAddress?: string; | ||
userCosmosAddress?: string; | ||
sortFiatValueDirection?: SortDirection; | ||
/** | ||
* If poolId is provided, only include assets that are part of the pool. | ||
*/ | ||
poolId?: string; | ||
} & AssetFilter): Promise<(TAsset & MaybeUserAssetCoin)[]> { | ||
const { userOsmoAddress, search, sortFiatValueDirection } = params; | ||
const { userCosmosAddress, search, sortFiatValueDirection } = params; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove redundant Boolean call It is not necessary to use the - if (!Boolean(userCosmosAddress)) return assets;
+ if (!userCosmosAddress) return assets;
|
||
let { assets } = params; | ||
if (!assets) assets = getAssets(params) as TAsset[]; | ||
|
||
|
@@ -87,11 +87,11 @@ export async function mapGetAssetsWithUserBalances< | |
) as TAsset[]; | ||
} | ||
|
||
if (!userOsmoAddress) return assets; | ||
if (!userCosmosAddress) return assets; | ||
|
||
const { balances } = await queryBalances({ | ||
...params, | ||
bech32Address: userOsmoAddress, | ||
bech32Address: userCosmosAddress, | ||
}); | ||
|
||
const eventualUserAssets = assets | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,7 +55,7 @@ export const assetsRouter = createTRPCRouter({ | |
return await getAssetWithUserBalance({ | ||
...ctx, | ||
asset, | ||
userOsmoAddress, | ||
userCosmosAddress: userOsmoAddress, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification Update all occurrences of The parameter name Files to update:
Analysis chainVerify the parameter name change. Ensure that all references to Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify all references to `userOsmoAddress` have been updated to `userCosmosAddress`.
# Test: Search for the old parameter name. Expect: No occurrences.
rg --type ts 'userOsmoAddress'
Length of output: 12383 |
||
}); | ||
} | ||
), | ||
|
@@ -87,7 +87,7 @@ export const assetsRouter = createTRPCRouter({ | |
mapGetAssetsWithUserBalances({ | ||
...ctx, | ||
search, | ||
userOsmoAddress, | ||
userCosmosAddress: userOsmoAddress, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification Update all references to The parameter name change from
Please ensure that all instances of Analysis chainVerify the parameter name change. Ensure that all references to Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify all references to `userOsmoAddress` have been updated to `userCosmosAddress`.
# Test: Search for the old parameter name. Expect: No occurrences.
rg --type ts 'userOsmoAddress'
Length of output: 12383 |
||
onlyVerified, | ||
sortFiatValueDirection: "desc", | ||
includePreview, | ||
|
@@ -165,7 +165,7 @@ export const assetsRouter = createTRPCRouter({ | |
const userAsset = await getAssetWithUserBalance({ | ||
...ctx, | ||
asset, | ||
userOsmoAddress, | ||
userCosmosAddress: userOsmoAddress, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification Update all references to The following instances of
Please update all the above references to Analysis chainVerify the parameter name change. Ensure that all references to Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify all references to `userOsmoAddress` have been updated to `userCosmosAddress`.
# Test: Search for the old parameter name. Expect: No occurrences.
rg --type ts 'userOsmoAddress'
Length of output: 12383 |
||
}); | ||
const userMarketAsset = await getMarketAsset({ | ||
asset: userAsset, | ||
|
@@ -290,7 +290,7 @@ export const assetsRouter = createTRPCRouter({ | |
...ctx, | ||
search, | ||
categories, | ||
userOsmoAddress, | ||
userCosmosAddress: userOsmoAddress, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification Inconsistent parameter name change detected The parameter
Please ensure that all references to Analysis chainVerify the parameter name change. Ensure that all references to Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify all references to `userOsmoAddress` have been updated to `userCosmosAddress`.
# Test: Search for the old parameter name. Expect: No occurrences.
rg --type ts 'userOsmoAddress'
Length of output: 12383 Tip Codebase Verification Update all references to The following files still contain references to
Please ensure that all occurrences are updated to maintain consistency across the codebase. Analysis chainVerify the parameter name change. Ensure that all references to Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify all references to `userOsmoAddress` have been updated to `userCosmosAddress`.
# Test: Search for the old parameter name. Expect: No occurrences.
rg --type ts 'userOsmoAddress'
Length of output: 12383 |
||
includePreview, | ||
}); | ||
|
||
|
@@ -543,7 +543,7 @@ export const assetsRouter = createTRPCRouter({ | |
...ctx, | ||
search, | ||
// Only get balances for withdraw | ||
userOsmoAddress: | ||
userCosmosAddress: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification Update all occurrences of The parameter name change has not been fully propagated throughout the codebase. Here are the locations that still reference
Please ensure that all instances of Analysis chainVerify the parameter name change. Ensure that all references to Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify all references to `userOsmoAddress` have been updated to `userCosmosAddress`.
# Test: Search for the old parameter name. Expect: No occurrences.
rg --type ts 'userOsmoAddress'
Length of output: 12383 |
||
type === "withdraw" ? userOsmoAddress : undefined, | ||
sortFiatValueDirection: "desc", | ||
includePreview, | ||
|
@@ -571,7 +571,7 @@ export const assetsRouter = createTRPCRouter({ | |
asset.variantGroupKey as (typeof variantsNotToBeExcluded)[number] | ||
) | ||
) { | ||
return asset.variantGroupKey === asset.coinDenom; | ||
return asset.variantGroupKey === asset.coinMinimalDenom; | ||
} | ||
|
||
return true; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add @deprecated tag for the import.
The import should be marked as deprecated to indicate that it will be removed in the future.
Committable suggestion