Skip to content

docs: fix memory layout comment in instructions sysvar#238

Merged
joncinque merged 2 commits intoanza-xyz:masterfrom
dhl:fix-instructions-sysvar-memory-layout-doc
Jul 24, 2025
Merged

docs: fix memory layout comment in instructions sysvar#238
joncinque merged 2 commits intoanza-xyz:masterfrom
dhl:fix-instructions-sysvar-memory-layout-doc

Conversation

@dhl
Copy link
Copy Markdown
Contributor

@dhl dhl commented Jul 18, 2025

This patch fixes the inaccuracies in the memory layout description for the serialize_instructions function.

Specifically, it addresses incorrect byte offsets (e.g. incorrect starting offset for table of sets) and not taking variable number of accounts into consideration.

Copilot AI review requested due to automatic review settings July 18, 2025 06:45
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes inaccuracies in the memory layout documentation for the serialize_instructions function in the instructions sysvar. The corrections address incorrect byte offsets and improve clarity by accounting for variable numbers of accounts in the serialization format.

Key changes:

  • Corrected byte offset calculations throughout the memory layout comment
  • Added explicit handling for variable account numbers in offset calculations
  • Improved comment structure to show relative offsets within instruction and account sections

Comment thread instructions-sysvar/src/lib.rs Outdated
@dhl dhl force-pushed the fix-instructions-sysvar-memory-layout-doc branch from e668339 to 68bf4d4 Compare July 18, 2025 06:50
Copy link
Copy Markdown
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.

Thanks for your contribution! This is definitely clearer, but I still had a slightly hard time with one part of it

Comment thread instructions-sysvar/src/lib.rs Outdated
Comment on lines +96 to +98
// 2 + (33 * num_accounts)..34 + (33 * num_accounts) - program_id - 32 bytes
// 34 + (33 * num_accounts)..36 + (33 * num_accounts) - data_len - 2 bytes
// 36 + (33 * num_accounts)..36 + (33 * num_accounts) + data_len - data - variable length
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: I had a bit of a hard time reading these, do you think this might be clearer? We could also use square brackets, but that might make things worse

Suggested change
// 2 + (33 * num_accounts)..34 + (33 * num_accounts) - program_id - 32 bytes
// 34 + (33 * num_accounts)..36 + (33 * num_accounts) - data_len - 2 bytes
// 36 + (33 * num_accounts)..36 + (33 * num_accounts) + data_len - data - variable length
// (2 + (33 * num_accounts))..(34 + (33 * num_accounts)) - program_id - 32 bytes
// (34 + (33 * num_accounts))..(36 + (33 * num_accounts)) - data_len - 2 bytes
// (36 + (33 * num_accounts))..(36 + (33 * num_accounts) + data_len) - data - variable length

Copy link
Copy Markdown
Contributor Author

@dhl dhl Jul 24, 2025

Choose a reason for hiding this comment

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

@joncinque thanks so much for reviewing this. I took your feedback and rewrote the instruction layout comment to improve clarity. I hope it is easier to follow now.

@dhl dhl marked this pull request as draft July 24, 2025 04:16
@dhl dhl force-pushed the fix-instructions-sysvar-memory-layout-doc branch from 23fc1f3 to e4f1e42 Compare July 24, 2025 05:36
@dhl dhl marked this pull request as ready for review July 24, 2025 05:38
@dhl dhl force-pushed the fix-instructions-sysvar-memory-layout-doc branch from e4f1e42 to a56f1e5 Compare July 24, 2025 05:45
Copy link
Copy Markdown
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.

Even better, thanks again!

@joncinque joncinque enabled auto-merge (squash) July 24, 2025 11:02
@joncinque joncinque merged commit 1afe23d into anza-xyz:master Jul 24, 2025
25 checks passed
@dhl dhl deleted the fix-instructions-sysvar-memory-layout-doc branch July 24, 2025 20:09
febo pushed a commit to febo/solana-sdk that referenced this pull request Sep 21, 2025
* docs: fix memory layout comment in instructions sysvar

* rewrite
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