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

Fixing misnamed field in PerformanceSample return type #3202

Merged
merged 2 commits into from
Sep 6, 2024

Conversation

disco-infinex
Copy link

PerformanceSample type, used to define the return values from the getRecentPerformanceSamples RPC Method incorrectly includes a numNonVoteTransaction field but the RPC call itself returns numNonVoteTransactions (with an "s").

In the core the fields called num_non_vote_transactions but you can see it should be mapped to numNonVoteTransactions, as tested here. The pluralise spelling is also consistent with the other fields (numSlots, numTransactions, etc.).

And, of course, numNonVoteTransactions is what's actually being returned. See for yourself:

curl https://api.devnet.solana.com -X POST -H "Content-Type: application/json" -s \
  -d '{"jsonrpc":"2.0", "id":1,"method": "getRecentPerformanceSamples","params": [1]}' | jq
{
  "jsonrpc": "2.0",
  "result": [
    {
      "numNonVoteTransactions": 850,
      "numSlots": 163,
      "numTransactions": 2792,
      "samplePeriodSecs": 60,
      "slot": 323584907
    }
  ],
  "id": 1
}

The docs are also wrong. I'll PR that too.

Copy link

changeset-bot bot commented Sep 4, 2024

🦋 Changeset detected

Latest commit: c64f650

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@mergify mergify bot added the community label Sep 4, 2024
@mergify mergify bot requested a review from a team September 4, 2024 06:55
Copy link
Contributor

@mcintyre94 mcintyre94 left a comment

Choose a reason for hiding this comment

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

Great spot, thankyou very much!

@disco-infinex
Copy link
Author

Looks like this failing test is being fixed in #3201

@mcintyre94
Copy link
Contributor

Perfect, thankyou!

@mcintyre94 mcintyre94 merged commit bf07a60 into solana-labs:master Sep 6, 2024
6 checks passed
@github-actions github-actions bot mentioned this pull request Sep 10, 2024
Copy link
Contributor

Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants