fix: do not throw when no env is found #29
Conversation
|
accidentally leaked env in first commit :/ revoked the key |
| @@ -29,19 +26,16 @@ import { | |||
|
|
|||
| export const ChainId = { | |||
There was a problem hiding this comment.
goerlies are dead, but some we are using were missing
| @@ -18,6 +18,7 @@ | |||
| "dist/lib.js", | |||
| "dist/lib.mjs" | |||
| ], | |||
| "bin": "./dist/cli.js", | |||
There was a problem hiding this comment.
adding cli bin so you can emit the env etc
| getRPCUrl, | ||
| } from "./lib"; | ||
|
|
||
| Object.keys(process.env).map((key) => delete process.env[key]); |
There was a problem hiding this comment.
i accidentally leaked the alchemy api key when updating the snapshots without noticing.
Running this before creating the snapshots, makes sure to cleanup the env.
| @@ -1,11 +1,13 @@ | |||
| import { networkMap } from "./alchemyIds"; | |||
| import { ChainId } from "./chainIds"; | |||
|
|
|||
| type SupportedChainIds = (typeof ChainId)[keyof typeof ChainId] & | |||
There was a problem hiding this comment.
This was a bit wrong.
SupportedChains was limited to "supported chains by alchemy", but it should be limited by supported chains by ChainId
| @@ -24,38 +26,69 @@ export const getNetworkEnv = (chainId: SupportedChainIds) => { | |||
| return env; | |||
| }; | |||
|
|
|||
| export function getExplicitRPC(chainId: SupportedChainIds) { | |||
There was a problem hiding this comment.
added explicit getters that throw - if you want alchemy and you can't get it i guess that is reasonable.
| export const getRPCUrl = (chainId: SupportedChainIds, alchemyKey?: string) => { | ||
| export const getRPCUrl = ( | ||
| chainId: SupportedChainIds, | ||
| options?: GetRPCUrlOptions, |
There was a problem hiding this comment.
changed to object, so we can add quicknode & drpc without introducing a breaking change.
| /** | ||
| * Return a RPC_URL for supported chains. | ||
| * If the RPC_URL environment variable is set, it will be used. | ||
| * Otherwise will construct the URL based on the chain ID and Alchemy API key. | ||
| * | ||
| * @notice This method acts as fall-through and will only revert if the ChainId is strictly not supported. |
There was a problem hiding this comment.
it's just easier to use in practice as right now what we do is fetch & catch & call with undefined - so as tools like viem have an internal fallback. Also makes the action behavior more reasonable.
Throwing when no env is found is more problematic then I thought.
If we have a .env with public RPCs and we read it before invoking the action, we'll end up using public rpcs instead of our private ones, which is suboptimal.
If we invoke the action after invoking the action, the action will revert on fork prs, because no secrets are exposed there (neither alchemy key, nor RPC_).
Not reverting is the only option on the action level and even inside ts, seems more optimal to fall-trough.