-
Notifications
You must be signed in to change notification settings - Fork 51
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
feat: Added API Keys #190
feat: Added API Keys #190
Conversation
pub(crate) fn new(rpc_addr: &str) -> Result<Self> { | ||
let connector = JsonRpcClient::new_client(); | ||
let rpc_client = connector.connect(rpc_addr); | ||
let mut rpc_client = connector.connect(rpc_addr); | ||
if let Ok(api_key) = std::env::var("NEAR_RPC_API_KEY") { | ||
let api_key = near_jsonrpc_client::auth::ApiKey::new(api_key) | ||
.map_err(|e| ErrorKind::DataConversion.custom(e))?; |
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.
Since we don't have the ability to override the RPC URL, this feels like a half-baked solution that won't be usable by anyone. I wonder if we should have a separate endpoint, like workspaces::mainnet
, except that it points to RPCaaS URL and requires an API key so this can't be misused. API keys are specific to that, so exposing the setting an API key for mainnet in general seems odd
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.
right, forgot that the Console will have it's own dedicated RPC URL instead of the defaults ones on *.near.org
, so thought this would be low hanging fruit to add like this. This goes into the territory of custom networks with all of its quirks, but it does seem easy to just add something like workspaces::custom
to point to a custom endpoint
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.
Yeah, or maybe providing reasoning for #103 so that this functionality could exist in a separate crate or any other community need. I feel like custom
might not solve this issue since API keys are only applicable to the new RPCaaS endpoints so would be janky to include with that API.
Either way, if we do introduce this ability, it should be an unstable
API as the API keys and endpoints might change in the future
Closing as it requires rebasing and addressing the review comments. Next attempt to resolve #184 should take all the above comments into account. |
PR adds in
NEAR_RPC_API_KEY
to interact with RPC related services. Resolves #184