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

fix: allow deserializing UiAccount rent epoch > u64 #2951

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jordy25519
Copy link

Problem

allows deserializing larger max rent epoch values

closes #2950

Summary of Changes

deserialize rent epoch as u128 before clamping to u64::MAX

@jordy25519
Copy link
Author

I'd also like to back port this fix to 1.16.* crates if possible

@buffalojoec
Copy link

buffalojoec commented Sep 30, 2024

Sorry @jordy25519 but what exactly is the motivation here? Everywhere in the source, rent_epoch is a u64. Also, we can't backport to 1.16. The oldest branch we could backport to would be 2.0, and you'd have to make a case for the backport.

EDIT: Oops, maybe I should read the linked issue. I'm still curious where that value comes from, though? It's 385 above u64::MAX.

@jordy25519
Copy link
Author

jordy25519 commented Oct 1, 2024

not sure right channel to broadcast to RPC providers and get this fixed as it shouldn't happen.
I assumed some intermediate infra is re-serializing the value incorrectly.
haven't seen the issue with rpcpool or helius, and given most clients are using Ts anyway the issue seems to have gone unnoticed for the most part.

for completeness, more examples:

{
  "id": 1,
  "jsonrpc": "2.0",
  "result": {
    "context": {
      "apiVersion": "1.18.22",
      "slot": 290645650
    },
    "value": {
      "data": [
        "KLUv/SAAAQAA",
        "base64+zstd"
      ],
      "executable": false,
      "lamports": 2088540689624,
      "owner": "11111111111111111111111111111111",
      "rentEpoch": 18446744073709552000,
      "space": 0
    }
  }
}

@jordy25519 jordy25519 closed this Oct 1, 2024
@jordy25519
Copy link
Author

jordy25519 commented Oct 1, 2024

accidentally closed

@jordy25519 jordy25519 reopened this Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

max rent epoch serialization
2 participants