-
Notifications
You must be signed in to change notification settings - Fork 35
chore(entropy) Dyamic Fee table #773
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Seems reasonable to me, left a couple comments. It would be great if you also run it by someone with better TS chops, maybe @cprussin ?
components/EntropyApiDataFetcher.tsx
Outdated
chain.reveal_delay_blocks !== 1 ? "s" : "" | ||
}`, | ||
gasLimit: chain.gas_limit.toLocaleString(), | ||
default_fee: chain.default_fee, |
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.
I don't think we should show this since it could confuse people or lead them to hardcode a value. Can we omit this or point them to the fee table with the correct dynamic values?
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.
We are not showing this at all. It's just there, not using this.
explorer?: string; | ||
} | ||
|
||
export const EntropyDeploymentsConfig: Record<string, ChainOverride> = { |
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.
maybe rename this to EntropyDeploymentsOverride or add a comment to communicate that this overrides the data from fortuna's chains/configs/ API?
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.
Mostly LGTM but some small things, biggest thing is to not abuse components where a hook should be used and to avoid typecasting
cc5b2ba
to
1514a45
Compare
* Update solana_mainnet.json ADD JUPSOL TO DOCS * Update solana_mainnet.json --------- Co-authored-by: Ali Behjati <[email protected]>
* chore(entropy) What's new * update * requested changes
Description
This PR take updated configs from fortuna
v1/chains/configs
and updates contract address table. I use network-id/chain-id to map with viem to get up-to-date explorer and RPC, hence removes the need of updating docs for new chains.Moreover, we can manually edit/add link and rpcs as some chains are still not supported by viem.
Type of Change
Areas Affected
Checklist
pre-commit run --all-files
to check for linting errorsRelated Issues
Closes #
Additional Notes
Contributor Information
Screenshots