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.

Sign up for GitHub

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

improve: allow canonical linking in deep-link #336

Merged
merged 7 commits into from
Oct 10, 2022
Merged

improve: allow canonical linking in deep-link #336

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
05e536f
feat: :sparkles: crate a mapping of string to numeric chain id
james-a-morris Sep 30, 2022
cd55d82
feat: :sparkles: add fns to resolve canonical name
james-a-morris Sep 30, 2022
9e689df
improve: enable canonical name linking in from/to
james-a-morris Sep 30, 2022
6fad86d
fix: remove typecast
james-a-morris Oct 3, 2022
f10db64
improve: hoist undefined logic into calling fn
james-a-morris Oct 3, 2022
2c79bf8
improve: add conditional
james-a-morris Oct 7, 2022
e0d9671
improve: add doc comment
james-a-morris Oct 7, 2022
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
2 changes: 1 addition & 1 deletion src/components/CoinSelection/useCoinSelection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export default function useCoinSelection() {
try {
// Check if Token exists and amount is convertable to Wei
config.getTokenInfoBySymbol(
Number(params.from),
config.resolveChainIdFromNumericOrCanonical(params.from),
params.asset.toUpperCase()
);
toWeiSafe(params.amount);
Expand Down
5 changes: 3 additions & 2 deletions src/hooks/useSendForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,8 +374,9 @@ function useSendFormManager(): SendFormManagerContext {
Because we need the asset's decimal value, you need to define **both** asset and amount for the optional params.
*/
useEffect(() => {
const fromChain = Number(params.from);
const toChain = Number(params.to);
const [fromChain, toChain] = [params.from, params.to].map(
config.resolveChainIdFromNumericOrCanonical
);
const areSupportedChains = [fromChain, toChain].every(
config.isSupportedChainId
);
Expand Down
17 changes: 17 additions & 0 deletions src/utils/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,23 @@ export class ConfigClient {
constants.isSupportedChainId(chainId) && this.spokeChains.has(chainId)
);
};
getSupportedCanonicalNameAsChainId = (canonicalName?: string) => {
// Returns undefined if the canonicalName is not defined
if (!canonicalName) return;
// Transform the canonical name to match ChainId key
const modifiedCanonicalName = canonicalName.toLowerCase();
// Attempt to resolve the chainId and return
const resolvedChain = constants.CanonicalChainName[modifiedCanonicalName];
return resolvedChain && this.isSupportedChainId(resolvedChain)
? resolvedChain
: undefined;
};
resolveChainIdFromNumericOrCanonical = (chainIdOrCanonical?: string) => {
return (
this.getSupportedCanonicalNameAsChainId(chainIdOrCanonical) ??
Copy link
Member

Choose a reason for hiding this comment

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

It might be better to use isNaN(chainIdOrCanonical) to first see if the string is a number. In which case we can parse as number.

If that fails then it must be a canonical name or a failure condition occurs.

I think the way you have it now, you're likely to try to parse invalid canonical chain names or empty strings as Number and that might give issues

Copy link
Contributor Author

@james-a-morris james-a-morris Oct 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

That's a good point!

I was trying to keep this function's logic as a stand-in for the original logic, which converts the to/from params to numbers directly by adding Number(...).

I will update it now and look for other improvements 😃

Copy link
Contributor Author

@james-a-morris james-a-morris Oct 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I've updated the function first to check if the chainIdOrCanonical variable is a numeric string. I've added doc comments to define how the function should return a NaN if an invalid case is found (non-numeric string & string does not contain a canonical name).
https://github.com/across-protocol/frontend-v2/blob/e0d9671544967f277f21f00f9812db2e7738b942/src/utils/config.ts#L153-L164

Number(chainIdOrCanonical)
);
};
// returns token list in order specified by constants, but adds in token address for the chain specified
getTokenList(chainId?: number): TokenList {
const routeTable = Object.fromEntries(
Expand Down
9 changes: 9 additions & 0 deletions src/utils/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ export enum ChainId {
// Polygon testnet
MUMBAI = 80001,
}

// Maps `ChainId` to an object and inverts the Key/Value
// pair. Ex) { "mainnet": 1 }
export const CanonicalChainName = Object.fromEntries(
Object.entries(ChainId)
.filter((v) => Number.isNaN(Number(v[0])))
.map((v) => [v[0].toLowerCase(), Number(v[1])])
);

/* Colors and Media Queries section */
export const BREAKPOINTS = {
tabletMin: 550,
Expand Down