improve: Suppress RPC provider key from error messages #1037
Conversation
pxrl
requested review from
mrice32,
nicholaspai and
james-a-morris
as code owners
There was a problem hiding this comment.
fwiw I am surprised how many instances we had of this in the code 🤔
Wish we could add a linting rule to prevent this somehow @james-a-morris any ideas? |
pxrl
force-pushed
the
pxrl/rpcKey
branch
2 times, most recently
from
dcad519
to
b50a0ba
Compare
Good idea on linting this! |
| function getProviderHostname(provider: ethers.providers.StaticJsonRpcProvider): string { | ||
| return new URL(provider.connection.url).hostname; | ||
| } |
There was a problem hiding this comment.
We have getOriginFromUrl( ) - should we introduce this second one? We can probably overload or make a similar function called getOriginFromProvider(provider: ...)
| // @dev To avoid accidentally leaking RPC keys in log messages, resolve the RPC provider hostname | ||
| // centrally. There should be no instances of `provider.connection.url` in log messages or errors. | ||
| function getProviderHostname(provider: ethers.providers.StaticJsonRpcProvider): string { | ||
| return new URL(provider.connection.url).hostname; |
There was a problem hiding this comment.
The hostname will clip away the network protocol & potential port. Using origin will futureproof us if we ever want to intermingle differing protocols or separate ports on the same host.
I think we should put all usages to |
No description provided.