Skip to content

Conversation

@febo
Copy link
Collaborator

@febo febo commented Jul 1, 2025

Problem

Currently the Rent sysvar includes a default implemention using the Default derive. This makes all values to be set to 0, which differs from the default values used in the SDK.

Solution

This PR adds a manual implementation for the Default trait with the expected default values.

Since the Default implementation is not strictly required for sysvars, this PR removes the requirement from the Sysvar trait and updates the sysvars implementation.

cc: @d0nutptr

@febo febo requested a review from joncinque July 1, 2025 13:30
Copy link
Collaborator

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Does this need a Default implementation at all? It's a bit dangerous to have it, especially because we're considering changing it for solana-foundation/solana-improvement-documents#194

I'd probably recommend removing it, but if it is needed, it should match the sdk's version

@febo
Copy link
Collaborator Author

febo commented Jul 1, 2025

Does this need a Default implementation at all? It's a bit dangerous to have it, especially because we're considering changing it for solana-foundation/solana-improvement-documents#194

I'd probably recommend removing it, but if it is needed, it should match the sdk's version

Good point! At the moment it is needed since the Sysvar trait requires it. But we can change that as well, since it is not really required.

I made the Sysvar change locally and everything else works.

@febo febo changed the title Add manual default implementation to Rent sysvar Remove Default implementation from sysvars Jul 2, 2025
@febo febo requested a review from joncinque July 2, 2025 12:47
Copy link
Collaborator

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks great!

@febo febo merged commit 3113862 into main Jul 10, 2025
9 checks passed
@febo febo deleted the febo/rent-default branch July 10, 2025 09:37
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.

3 participants