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.

Sign up for GitHub

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

Jump to bottom

fix: add versioned params e2e's test #248

Merged
merged 8 commits into from
May 19, 2023
Merged

fix: add versioned params e2e's test #248

merged 8 commits into from
May 19, 2023

Conversation

jingjunLi
Copy link
Contributor

@jingjunLi jingjunLi commented May 16, 2023 •
edited
Loading

Description

  • VersionedParams RPC API to query versionedParams according to timestamp.
  • add versioned params e2e's test

Rationale

tell us why we need these changes...

Example

blockTime := ctx.BlockTime().Unix()
res err := k.VersionedParams(ctx, &types.QueryVersionedParamsRequest{Timestamp: blockTime})

Changes

Notable changes:

  • add each change in a bullet point here
  • ...

@jingjunLi jingjunLi added the wip label May 16, 2023
@jingjunLi jingjunLi added r4r and removed wip labels May 16, 2023
@jingjunLi jingjunLi requested review from fynnss and RenRick May 16, 2023 08:16
fynnss
fynnss reviewed May 17, 2023
View reviewed changes
x/storage/keeper/grpc_query.go Outdated
ctx := sdk.UnwrapSDKContext(c)

params := k.GetParams(ctx)
versionedParams, err := k.GetVersionedParamsWithTs(ctx, req.GetTimestamp())
Copy link
Contributor

Choose a reason for hiding this comment

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

If the timestamp is zero or not-specify, do we need to return the latest params?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I have added it.

fynnss
fynnss approved these changes May 17, 2023
View reviewed changes
Copy link
Contributor

@fynnss fynnss left a comment

Choose a reason for hiding this comment

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

LGTM

@unclezoro unclezoro merged commit abb09df into develop May 19, 2023
@unclezoro unclezoro mentioned this pull request May 23, 2023
Merged
forcodedancing pushed a commit that referenced this pull request May 25, 2023
@jingjunLi @forcodedancing
fix: add versioned params e2e's test (#248)
9ea1747
@unclezoro unclezoro deleted the chris/versioned-params-e2e branch August 16, 2023 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fynnss fynnss fynnss approved these changes

@RenRick RenRick Awaiting requested review from RenRick

Assignees
No one assigned
Labels
r4r
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jingjunLi @fynnss @unclezoro