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.

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(rpc): add protocol config rpc endpoint #3919

Merged
merged 6 commits into from
Feb 10, 2021
Merged

Conversation

bowenwang1996
Copy link
Collaborator

@bowenwang1996 bowenwang1996 commented Feb 5, 2021

Add EXPERIMENTAL_protocol_config rpc endpoint that returns the protocol config given some block, since protocol config can change after genesis due to protocol upgrades. This will allow users to get the up-to-date protocol-level configs. Fixes #3918.

Test plan

test_protocol_config_rpc

@bowenwang1996 bowenwang1996 marked this pull request as ready for review February 7, 2021 19:22
chain/jsonrpc-primitives/src/types/config.rs Outdated Show resolved Hide resolved
chain/jsonrpc-primitives/src/types/config.rs Outdated Show resolved Hide resolved
Co-authored-by: Bohdan Khorolets <b@khorolets.com>
chain/jsonrpc-primitives/src/types/config.rs Outdated Show resolved Hide resolved
/// Current Protocol Version
pub protocol_version: ProtocolVersion,
/// Official time of blockchain start.
pub genesis_time: DateTime<Utc>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to make sure that dates, u64-like types (BlockHeight) are serialized as strings due to JSON numbers limitation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe it is. For example,

"genesis_time": "1970-01-01T00:00:00.000000000Z",

Co-authored-by: Vlad Frolov <frolvlad@gmail.com>
@bowenwang1996
Copy link
Collaborator Author

@frol do you think we should deprecate EXPERIMENTAL_genesis_config (make it an alias to EXPERIMENTAL_protocol_config)? Or should we introduce EXPERIMENTAL_protocol_config separately in near-api-js? If we do the latter, we also need to make sure that wallet uses the new endpoint.

@frol
Copy link
Collaborator

frol commented Feb 10, 2021

@bowenwang1996 Good point. While the naming geneis_config implies that it should be genesis config, I think it makes sense to actually return the latest runtime config since that is what most of the users wanted to have in the first place; thus, I think we should go with the alias 👍

@frol
Copy link
Collaborator

frol commented Feb 10, 2021

Well, on the second thought, it would not be sane to expose the latest protocol config under genesis_config endpoint, and it is better to address the issue on near-api-js level (add deprecation note on the method, and use EXPERIMENTAL_protocol_config({"sync_checkpoint": "genesis"}) explicitly). I will take it from here.

@bowenwang1996
Copy link
Collaborator Author

Well, on the second thought, it would not be sane to expose the latest protocol config under genesis_config endpoint, and it is better to address the issue on near-api-js level (add deprecation note on the method, and use EXPERIMENTAL_protocol_config({"sync_checkpoint": "genesis"}) explicitly). I will take it from here.

Thanks @frol !

@frol frol added the automerge label Feb 10, 2021
@near-bulldozer near-bulldozer bot merged commit 8dadb05 into master Feb 10, 2021
@near-bulldozer near-bulldozer bot deleted the protocol-config-rpc branch February 10, 2021 18:34
bowenwang1996 added a commit that referenced this pull request Feb 18, 2021
Add `EXPERIMENTAL_protocol_config` rpc endpoint that returns the protocol config given some block, since protocol config can change after genesis due to protocol upgrades. This will allow users to get the up-to-date protocol-level configs. Fixes #3918.

Test plan
----------
`test_protocol_config_rpc`
@frol
Copy link
Collaborator

frol commented Feb 20, 2021

Just for the reference, near-api-js support is coming: near/near-api-js#508

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RPC for getting the current config
4 participants