-
Couldn't load subscription status.
- Fork 40
Adding ShankAccounts Macro #76
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
base: master
Are you sure you want to change the base?
Conversation
Summary by CodeRabbit
WalkthroughIntroduces a new ShankAccounts derive macro and end-to-end integration: macro parsing/generation, IDL extraction/expansion for struct-based accounts, a generic Context type, feature-gated Solana support, examples, and comprehensive tests. IDL generation now resolves #[accounts(StructName)] by expanding fields, with fallback to placeholder when unresolved. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Parser as Rust Parser
participant Extract as extract_shank_accounts_structs
participant Map as Accounts Map (HashMap)
participant Enum as Instruction Enum Parser
participant IDL as IdlInstruction::try_from_with_accounts
Parser->>Extract: Iterate ItemStructs
Extract->>Map: Insert StructName -> Vec<InstructionAccount>
Parser->>Enum: Parse InstructionVariant + attributes
Enum->>IDL: Try convert variant with accounts_map
alt Inline accounts
IDL->>IDL: Map inline accounts to IdlAccountItems
else Struct accounts
IDL->>Map: Lookup StructName
alt Found
Map-->>IDL: Vec<InstructionAccount>
IDL->>IDL: Expand to IdlAccountItems
else Not found
IDL->>IDL: Emit placeholder account item
end
end
IDL-->>Parser: IdlInstruction(s)
sequenceDiagram
autonumber
participant Caller as Program Logic
participant Gen as ShankAccounts::context()
participant Ctx as Context<'a, T, U>
note over Gen: Generated by ShankAccounts derive
Caller->>Gen: context(&[AccountInfo])
Gen->>Gen: Validate min accounts length
Gen->>Gen: For Option<&AccountInfo>: if key == crate::ID then None else Some(&acc[i])
Gen->>Ctx: Construct Context { accounts: T, remaining_accounts }
Ctx-->>Caller: Typed access (ctx.accounts), remaining_accounts
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 81
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
shank-idl/src/idl_instruction.rs (1)
264-273: Same off-by-one in the accounts-aware path.Mirror the fix in try_from_with_accounts.
- ensure!( - discriminant < u8::MAX as usize, + ensure!( + discriminant <= u8::MAX as usize, anyhow!( "Instruction variant discriminants have to be <= u8::MAX ({}), \ but the discriminant of variant '{}' is {}", u8::MAX, ident, discriminant ) );shank-macro-impl/src/instruction/account_attrs.rs (1)
222-249: Index-position validation is good; consider duplicate-name detection.Multiple inline accounts with the same name should error early to avoid confusing IDL.
If desired, I can add a small pass to check for duplicate InstructionAccount.name values and return a ParseError.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (61)
EXAMPLE_USAGE.md(1 hunks)shank-idl/ACCOUNTS_STRUCT_IDL.md(1 hunks)shank-idl/src/accounts_extraction.rs(1 hunks)shank-idl/src/file.rs(3 hunks)shank-idl/src/idl_instruction.rs(4 hunks)shank-idl/src/lib.rs(1 hunks)shank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.json(1 hunks)shank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.rs(1 hunks)shank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.json(1 hunks)shank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.rs(1 hunks)shank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.json(1 hunks)shank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.rs(1 hunks)shank-idl/tests/generate_expected_idl.rs(1 hunks)shank-idl/tests/instructions.rs(1 hunks)shank-idl/tests/test_shank_accounts_expansion.rs(1 hunks)shank-macro-impl/src/accounts/account_field.rs(1 hunks)shank-macro-impl/src/accounts/account_struct.rs(1 hunks)shank-macro-impl/src/accounts/mod.rs(1 hunks)shank-macro-impl/src/instruction/account_attrs.rs(3 hunks)shank-macro-impl/src/instruction/instruction.rs(3 hunks)shank-macro-impl/src/lib.rs(2 hunks)shank-macro/src/accounts.rs(1 hunks)shank-macro/src/lib.rs(4 hunks)shank/Cargo.toml(1 hunks)shank/examples/accounts_struct_example.rs(1 hunks)shank/examples/anchor_style_example.rs(1 hunks)shank/examples/context_usage_example.rs(1 hunks)shank/examples/full_solana_example.rs(1 hunks)shank/src/context.rs(1 hunks)shank/src/lib.rs(1 hunks)shank/tests/accounts_macro_test.rs(1 hunks)shank/tests/basic_context_test.rs(1 hunks)shank/tests/context_api_test.rs(1 hunks)shank/tests/context_api_working_test.rs(1 hunks)shank/tests/context_generation_test.rs(1 hunks)shank/tests/context_issue_demo.rs(1 hunks)shank/tests/context_method_demo.rs(1 hunks)shank/tests/context_method_test.rs(1 hunks)shank/tests/context_runtime_test.rs(1 hunks)shank/tests/context_usage_test.rs(1 hunks)shank/tests/enhanced_accounts_test.rs(1 hunks)shank/tests/error_handling_test.rs(1 hunks)shank/tests/expand/basic_accounts.expanded.rs(1 hunks)shank/tests/expand/basic_accounts.rs(1 hunks)shank/tests/expand/complex_constraints.expanded.rs(1 hunks)shank/tests/expand/complex_constraints.rs(1 hunks)shank/tests/expand/custom_lifetime.expanded.rs(1 hunks)shank/tests/expand/custom_lifetime.rs(1 hunks)shank/tests/expand/empty_struct.expanded.rs(1 hunks)shank/tests/expand/empty_struct.rs(1 hunks)shank/tests/expand/no_constraints.expanded.rs(1 hunks)shank/tests/expand/no_constraints.rs(1 hunks)shank/tests/expand/optional_accounts.expanded.rs(1 hunks)shank/tests/expand/optional_accounts.rs(1 hunks)shank/tests/expand_tests.rs(1 hunks)shank/tests/final_context_test.rs(1 hunks)shank/tests/idl_generation_test.rs(1 hunks)shank/tests/instruction_integration_test.rs(1 hunks)shank/tests/instruction_with_accounts_struct_test.rs(1 hunks)shank/tests/simple_context_test.rs(1 hunks)shank/tests/working_context_demo.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files and fixtures under each crate’s tests/ directory
Files:
shank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.jsonshank/tests/expand_tests.rsshank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.jsonshank/tests/expand/empty_struct.rsshank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.jsonshank-idl/tests/generate_expected_idl.rsshank/tests/expand/empty_struct.expanded.rsshank/tests/expand/basic_accounts.rsshank-idl/tests/test_shank_accounts_expansion.rsshank/tests/working_context_demo.rsshank-idl/tests/instructions.rsshank/tests/expand/complex_constraints.rsshank/tests/basic_context_test.rsshank/tests/simple_context_test.rsshank/tests/expand/custom_lifetime.rsshank/tests/context_method_demo.rsshank/tests/instruction_with_accounts_struct_test.rsshank/tests/expand/basic_accounts.expanded.rsshank/tests/final_context_test.rsshank/tests/expand/optional_accounts.rsshank/tests/expand/complex_constraints.expanded.rsshank/tests/idl_generation_test.rsshank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.rsshank/tests/expand/no_constraints.rsshank/tests/context_runtime_test.rsshank/tests/error_handling_test.rsshank/tests/context_api_working_test.rsshank/tests/instruction_integration_test.rsshank/tests/accounts_macro_test.rsshank/tests/context_method_test.rsshank/tests/expand/optional_accounts.expanded.rsshank/tests/expand/custom_lifetime.expanded.rsshank/tests/context_generation_test.rsshank/tests/context_api_test.rsshank/tests/context_issue_demo.rsshank/tests/context_usage_test.rsshank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.rsshank/tests/enhanced_accounts_test.rsshank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.rsshank/tests/expand/no_constraints.expanded.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format all Rust source files according to rustfmt.toml
Use #[derive(ShankAccount)] to mark account structs; optionally annotate with #[seeds] for PDA generation
Use #[derive(ShankInstruction)] to mark instruction enums; use #[account] attributes on fields as needed
Use #[derive(ShankType)] to mark custom types for IDL inclusion
Use #[derive(ShankBuilder)] to generate instruction builders
Use #[derive(ShankContext)] to generate account context structs
Use #[padding] on fields to mark padding in the generated IDL
Use #[idl_type("TypeName")] to override a field’s IDL type
Use #[idl_name("name")] to rename a field in the IDL while keeping the Rust field name
Use #[skip] on fields to exclude them from the generated IDL
Files:
shank/tests/expand_tests.rsshank-macro-impl/src/lib.rsshank/tests/expand/empty_struct.rsshank/src/context.rsshank/src/lib.rsshank-macro/src/accounts.rsshank-idl/tests/generate_expected_idl.rsshank-macro-impl/src/accounts/account_struct.rsshank/tests/expand/empty_struct.expanded.rsshank/tests/expand/basic_accounts.rsshank-idl/src/lib.rsshank-idl/tests/test_shank_accounts_expansion.rsshank-idl/src/accounts_extraction.rsshank/tests/working_context_demo.rsshank-idl/tests/instructions.rsshank/examples/accounts_struct_example.rsshank/tests/expand/complex_constraints.rsshank/examples/context_usage_example.rsshank/tests/basic_context_test.rsshank/tests/simple_context_test.rsshank/tests/expand/custom_lifetime.rsshank/tests/context_method_demo.rsshank/tests/instruction_with_accounts_struct_test.rsshank/tests/expand/basic_accounts.expanded.rsshank-macro-impl/src/accounts/mod.rsshank/tests/final_context_test.rsshank/tests/expand/optional_accounts.rsshank-macro-impl/src/instruction/instruction.rsshank/tests/expand/complex_constraints.expanded.rsshank/tests/idl_generation_test.rsshank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.rsshank/tests/expand/no_constraints.rsshank/tests/context_runtime_test.rsshank/tests/error_handling_test.rsshank-macro-impl/src/accounts/account_field.rsshank/tests/context_api_working_test.rsshank/tests/instruction_integration_test.rsshank-idl/src/file.rsshank/tests/accounts_macro_test.rsshank/tests/context_method_test.rsshank/tests/expand/optional_accounts.expanded.rsshank/tests/expand/custom_lifetime.expanded.rsshank-idl/src/idl_instruction.rsshank/tests/context_generation_test.rsshank/tests/context_api_test.rsshank/tests/context_issue_demo.rsshank/tests/context_usage_test.rsshank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.rsshank/examples/anchor_style_example.rsshank/tests/enhanced_accounts_test.rsshank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.rsshank-macro-impl/src/instruction/account_attrs.rsshank/examples/full_solana_example.rsshank-macro/src/lib.rsshank/tests/expand/no_constraints.expanded.rs
**/Cargo.toml
📄 CodeRabbit inference engine (CLAUDE.md)
Set Rust edition to 2018 in all crates (edition = "2018" in Cargo.toml)
Files:
shank/Cargo.toml
🧬 Code graph analysis (49)
shank-macro-impl/src/lib.rs (1)
shank-idl/src/file.rs (1)
accounts(99-108)
shank/src/lib.rs (1)
shank/tests/context_api_working_test.rs (1)
key(14-16)
shank-macro/src/accounts.rs (1)
shank-macro-impl/src/accounts/mod.rs (1)
try_from(21-48)
shank-idl/tests/generate_expected_idl.rs (2)
shank-idl/src/file.rs (2)
parse_file(64-97)optional_program_address(51-56)shank-idl/src/idl.rs (1)
try_into_json(64-67)
shank-macro-impl/src/accounts/account_struct.rs (1)
shank-macro-impl/src/accounts/account_field.rs (1)
from_field(15-76)
shank/tests/expand/empty_struct.expanded.rs (5)
shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/expand/complex_constraints.expanded.rs (1)
__shank_accounts(23-85)shank/tests/expand/custom_lifetime.expanded.rs (1)
__shank_accounts(15-41)shank/tests/expand/no_constraints.expanded.rs (1)
__shank_accounts(14-25)shank/tests/expand/optional_accounts.expanded.rs (1)
__shank_accounts(17-52)
shank/tests/expand/basic_accounts.rs (2)
shank/examples/accounts_struct_example.rs (1)
main(103-150)shank/examples/anchor_style_example.rs (2)
main(113-127)main(130-132)
shank-idl/tests/test_shank_accounts_expansion.rs (2)
shank-idl/src/lib.rs (2)
extract_idl(44-63)default(32-38)shank-idl/src/file.rs (3)
default(39-47)instructions(110-130)accounts(99-108)
shank-idl/src/accounts_extraction.rs (3)
shank-macro-impl/src/parsers/attrs.rs (1)
get_derive_attr(74-79)shank-idl/src/file.rs (1)
accounts(99-108)shank-idl/src/idl_field.rs (1)
field(35-39)
shank/tests/working_context_demo.rs (3)
shank/examples/accounts_struct_example.rs (1)
main(103-150)shank/examples/context_usage_example.rs (2)
main(95-113)main(116-119)shank/tests/expand/basic_accounts.expanded.rs (2)
main(54-54)__shank_accounts(17-52)
shank-idl/tests/instructions.rs (1)
shank-idl/src/file.rs (2)
parse_file(64-97)optional_program_address(51-56)
shank/examples/accounts_struct_example.rs (4)
shank-idl/src/file.rs (1)
accounts(99-108)shank/examples/anchor_style_example.rs (2)
main(113-127)main(130-132)shank/examples/context_usage_example.rs (2)
main(95-113)main(116-119)shank/examples/full_solana_example.rs (2)
main(131-142)main(145-159)
shank/tests/expand/complex_constraints.rs (2)
shank/examples/accounts_struct_example.rs (1)
main(103-150)shank/examples/anchor_style_example.rs (2)
main(113-127)main(130-132)
shank/examples/context_usage_example.rs (3)
shank/examples/accounts_struct_example.rs (1)
main(103-150)shank/examples/anchor_style_example.rs (2)
main(113-127)main(130-132)shank/examples/full_solana_example.rs (2)
main(131-142)main(145-159)
shank/tests/basic_context_test.rs (2)
shank/src/lib.rs (1)
key(10-10)shank/tests/context_api_working_test.rs (1)
key(14-16)
shank/tests/simple_context_test.rs (1)
shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)
shank/tests/expand/custom_lifetime.rs (3)
shank/examples/accounts_struct_example.rs (1)
main(103-150)shank/examples/anchor_style_example.rs (2)
main(113-127)main(130-132)shank/examples/full_solana_example.rs (2)
main(131-142)main(145-159)
shank/tests/context_method_demo.rs (1)
shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)
shank/tests/instruction_with_accounts_struct_test.rs (1)
shank-idl/src/file.rs (1)
accounts(99-108)
shank/tests/expand/basic_accounts.expanded.rs (5)
shank/tests/expand/complex_constraints.expanded.rs (1)
__shank_accounts(23-85)shank/tests/expand/custom_lifetime.expanded.rs (1)
__shank_accounts(15-41)shank/tests/expand/empty_struct.expanded.rs (1)
__shank_accounts(5-9)shank/tests/expand/no_constraints.expanded.rs (1)
__shank_accounts(14-25)shank/tests/expand/optional_accounts.expanded.rs (1)
__shank_accounts(17-52)
shank-macro-impl/src/accounts/mod.rs (3)
shank-macro-impl/src/accounts/account_struct.rs (1)
from_fields(12-23)shank/src/lib.rs (1)
key(10-10)shank/tests/context_api_working_test.rs (1)
key(14-16)
shank/tests/final_context_test.rs (2)
shank/src/lib.rs (1)
key(10-10)shank/tests/context_api_working_test.rs (1)
key(14-16)
shank/tests/expand/optional_accounts.rs (3)
shank/examples/accounts_struct_example.rs (1)
main(103-150)shank/examples/anchor_style_example.rs (2)
main(113-127)main(130-132)shank/examples/full_solana_example.rs (2)
main(131-142)main(145-159)
shank-macro-impl/src/instruction/instruction.rs (2)
shank-idl/src/idl_instruction.rs (2)
try_from(22-29)try_from(67-165)shank-macro-impl/src/instruction/account_attrs.rs (3)
try_from(193-219)try_from(225-248)attrs(226-230)
shank/tests/expand/complex_constraints.expanded.rs (4)
shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/expand/custom_lifetime.expanded.rs (1)
__shank_accounts(15-41)shank/tests/expand/no_constraints.expanded.rs (1)
__shank_accounts(14-25)shank/tests/expand/optional_accounts.expanded.rs (1)
__shank_accounts(17-52)
shank/tests/idl_generation_test.rs (6)
shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/expand/complex_constraints.expanded.rs (1)
__shank_accounts(23-85)shank/tests/expand/custom_lifetime.expanded.rs (1)
__shank_accounts(15-41)shank/tests/expand/empty_struct.expanded.rs (1)
__shank_accounts(5-9)shank/tests/expand/no_constraints.expanded.rs (1)
__shank_accounts(14-25)shank/tests/expand/optional_accounts.expanded.rs (1)
__shank_accounts(17-52)
shank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.rs (1)
shank-idl/src/file.rs (1)
accounts(99-108)
shank/tests/expand/no_constraints.rs (2)
shank/examples/accounts_struct_example.rs (1)
main(103-150)shank/examples/anchor_style_example.rs (2)
main(113-127)main(130-132)
shank/tests/context_runtime_test.rs (6)
shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/expand/complex_constraints.expanded.rs (1)
__shank_accounts(23-85)shank/tests/expand/custom_lifetime.expanded.rs (1)
__shank_accounts(15-41)shank/tests/expand/empty_struct.expanded.rs (1)
__shank_accounts(5-9)shank/tests/expand/no_constraints.expanded.rs (1)
__shank_accounts(14-25)shank/tests/expand/optional_accounts.expanded.rs (1)
__shank_accounts(17-52)
shank/tests/error_handling_test.rs (7)
shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/expand/complex_constraints.expanded.rs (1)
__shank_accounts(23-85)shank/tests/expand/custom_lifetime.expanded.rs (1)
__shank_accounts(15-41)shank/tests/expand/empty_struct.expanded.rs (1)
__shank_accounts(5-9)shank/tests/expand/no_constraints.expanded.rs (1)
__shank_accounts(14-25)shank/tests/expand/optional_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/context_api_test.rs (1)
test_different_lifetime_names(83-97)
shank/tests/context_api_working_test.rs (1)
shank/src/lib.rs (1)
key(10-10)
shank/tests/instruction_integration_test.rs (6)
shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/expand/complex_constraints.expanded.rs (1)
__shank_accounts(23-85)shank/tests/expand/custom_lifetime.expanded.rs (1)
__shank_accounts(15-41)shank/tests/expand/empty_struct.expanded.rs (1)
__shank_accounts(5-9)shank/tests/expand/no_constraints.expanded.rs (1)
__shank_accounts(14-25)shank/tests/expand/optional_accounts.expanded.rs (1)
__shank_accounts(17-52)
shank-idl/src/file.rs (2)
shank-idl/src/accounts_extraction.rs (1)
extract_shank_accounts_structs(12-26)shank-idl/src/idl_instruction.rs (3)
try_into_with_accounts(33-43)ix(23-27)ix(37-41)
shank/tests/context_method_test.rs (3)
shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/expand/optional_accounts.expanded.rs (1)
__shank_accounts(17-52)shank-macro-impl/src/parsed_struct/struct_attr.rs (1)
len(81-83)
shank/tests/expand/optional_accounts.expanded.rs (5)
shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/expand/complex_constraints.expanded.rs (1)
__shank_accounts(23-85)shank/tests/expand/custom_lifetime.expanded.rs (1)
__shank_accounts(15-41)shank/tests/expand/empty_struct.expanded.rs (1)
__shank_accounts(5-9)shank/tests/expand/no_constraints.expanded.rs (1)
__shank_accounts(14-25)
shank/tests/expand/custom_lifetime.expanded.rs (3)
shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/expand/empty_struct.expanded.rs (1)
__shank_accounts(5-9)shank/tests/expand/no_constraints.expanded.rs (1)
__shank_accounts(14-25)
shank-idl/src/idl_instruction.rs (3)
shank-idl/src/file.rs (2)
instructions(110-130)accounts(99-108)shank-macro-impl/src/instruction/account_attrs.rs (2)
try_from(193-219)try_from(225-248)shank-macro-impl/src/instruction/instruction.rs (3)
try_from(50-57)try_from(63-76)try_from(101-177)
shank/tests/context_generation_test.rs (6)
shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/expand/complex_constraints.expanded.rs (1)
__shank_accounts(23-85)shank/tests/expand/custom_lifetime.expanded.rs (1)
__shank_accounts(15-41)shank/tests/expand/empty_struct.expanded.rs (1)
__shank_accounts(5-9)shank/tests/expand/no_constraints.expanded.rs (1)
__shank_accounts(14-25)shank/tests/expand/optional_accounts.expanded.rs (1)
__shank_accounts(17-52)
shank/tests/context_api_test.rs (7)
shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/expand/complex_constraints.expanded.rs (1)
__shank_accounts(23-85)shank/tests/expand/custom_lifetime.expanded.rs (1)
__shank_accounts(15-41)shank/tests/expand/empty_struct.expanded.rs (1)
__shank_accounts(5-9)shank/tests/expand/no_constraints.expanded.rs (1)
__shank_accounts(14-25)shank/tests/expand/optional_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/error_handling_test.rs (1)
test_different_lifetime_names(175-196)
shank/tests/context_issue_demo.rs (2)
shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/expand/optional_accounts.expanded.rs (1)
__shank_accounts(17-52)
shank/tests/context_usage_test.rs (4)
shank/src/lib.rs (1)
key(10-10)shank/tests/context_api_working_test.rs (1)
key(14-16)shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/expand/complex_constraints.expanded.rs (1)
__shank_accounts(23-85)
shank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.rs (1)
shank-idl/src/file.rs (1)
accounts(99-108)
shank/examples/anchor_style_example.rs (4)
shank-idl/src/file.rs (3)
accounts(99-108)ctx(147-151)ctx(153-157)shank/examples/accounts_struct_example.rs (1)
main(103-150)shank/examples/context_usage_example.rs (2)
main(95-113)main(116-119)shank/examples/full_solana_example.rs (2)
main(131-142)main(145-159)
shank/tests/enhanced_accounts_test.rs (1)
shank-idl/src/file.rs (1)
accounts(99-108)
shank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.rs (1)
shank-idl/src/file.rs (1)
accounts(99-108)
shank-macro-impl/src/instruction/account_attrs.rs (2)
shank-idl/src/idl_instruction.rs (2)
try_from(22-29)try_from(67-165)shank-macro-impl/src/instruction/instruction.rs (3)
try_from(50-57)try_from(63-76)try_from(101-177)
shank/examples/full_solana_example.rs (6)
shank-idl/src/file.rs (3)
accounts(99-108)ctx(147-151)ctx(153-157)shank/src/lib.rs (1)
key(10-10)shank/tests/context_api_working_test.rs (2)
key(14-16)main(116-118)shank/examples/accounts_struct_example.rs (1)
main(103-150)shank/examples/anchor_style_example.rs (2)
main(113-127)main(130-132)shank/examples/context_usage_example.rs (2)
main(95-113)main(116-119)
shank-macro/src/lib.rs (2)
shank-idl/src/file.rs (1)
accounts(99-108)shank-macro/src/accounts.rs (1)
derive_accounts(6-9)
shank/tests/expand/no_constraints.expanded.rs (3)
shank/tests/expand/basic_accounts.expanded.rs (1)
__shank_accounts(17-52)shank/tests/expand/complex_constraints.expanded.rs (1)
__shank_accounts(23-85)shank/tests/expand/empty_struct.expanded.rs (1)
__shank_accounts(5-9)
🪛 markdownlint-cli2 (0.18.1)
shank-idl/ACCOUNTS_STRUCT_IDL.md
130-130: Files should end with a single newline character
(MD047, single-trailing-newline)
🔇 Additional comments (68)
shank/Cargo.toml (2)
13-20: Feature-gated Solana dep looks correct.Optional solana-program dependency and feature mapping via dep:solana-program are set up properly; edition = "2018" complies with guidelines.
13-20: No change required — feature name is consistently hyphenated ("solana-program").
Search results show only cfg(feature = "solana-program") usages; no "solana_program" occurrences found.shank/tests/context_method_test.rs (1)
29-47: Confirm “optional” flag position (tuple index 5) is stable.If the tuple shape evolves, this test will silently mis-read fields. Consider asserting the field name too to anchor expectations.
shank/tests/context_method_demo.rs (3)
50-64: Good coverage of IDL flags.The assertions validate mut, signer, and optional semantics clearly.
93-107: Mirror the same IDL assertions in the no-feature path.Consistent and clear; keeps behavior aligned across cfgs.
25-48: Unify context() signature in docs/examples to match the macro APIRepo contains mixed examples using:
- context(&accounts) / context(accounts)
- context(accounts, program_id)
- context(program_id, accounts)
Update all comments/examples to the single canonical signature the macro actually generates. Representative locations to fix: shank-macro/src/lib.rs (~lines 345 and 434), EXAMPLE_USAGE.md:34, shank/examples/full_solana_example.rs:87/154, shank/examples/context_usage_example.rs:45, shank/tests/context_usage_test.rs:77/151/225, shank/tests/context_method_demo.rs:35.
shank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.json (1)
14-17: Confirm expected IDL field casing.The account "dataAccount" (camelCase) may not match default generation from a Rust field named data_account unless #[idl_name("dataAccount")] is used. Align casing or annotate the Rust source.
shank/tests/expand/empty_struct.rs (1)
3-6: LGTM: empty struct expansion fixture.Fits macrotest pattern; main() stub is appropriate for expansion.
shank/tests/instruction_with_accounts_struct_test.rs (1)
80-89: No change required — IDL names are camelCase by default. The fixture pair shows Rust fielddata_account-> IDL"dataAccount"(shank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.rs and .json); only add#[idl_name("dataAccount")]if you want to override the default conversion.shank-macro-impl/src/lib.rs (2)
2-2: New accounts module export matches the added derive surfaceGood addition. Ensure all internal references use this module path consistently.
17-17: Proc-macro entrypoint wired: ShankAccounts → derive_accountsVerified — shank-macro/src/lib.rs contains #[proc_macro_derive(ShankAccounts)] and pub fn shank_accounts(...) which calls derive_accounts(...). (shank-macro/src/lib.rs:447–452)
shank-idl/src/lib.rs (1)
8-8: Private accounts_extraction module inclusionLGTM. Keep it private unless there’s a clear external consumer to avoid API surface creep.
shank/tests/expand/empty_struct.expanded.rs (1)
1-11: Snapshot contents look sane for the empty-accounts caseNo issues. Keep in mind snapshots can be toolchain‑sensitive; update fixtures if rustc output changes.
If you hit frequent snapshot churn, consider macrotest filters to strip unstable attrs like
#[rustc_box].shank/tests/expand/no_constraints.rs (1)
10-15: No-constraints derive case: OKGood fixture for read-only defaults without attributes.
shank/src/lib.rs (1)
4-5: Inspectcontextmodule for Solana importsPlease check the output of the above script to confirm whether
shank/src/context.rsimports any Solana crates. Once we verify there are no Solana dependencies, we can remove the feature‐gate suggestion and consider this re‐export safe.shank-macro/src/accounts.rs (1)
6-9: Bridge is correct — proc‑macro maps errors to compile_error()
Confirmed: shank-macro/src/lib.rs —shank_accountscallsderive_accounts(input).unwrap_or_else(to_compile_error).into()andto_compile_errordelegates toParseError::to_compile_error(...).shank/tests/expand/no_constraints.expanded.rs (1)
12-25: Accounts metadata looks correct for a no-constraints struct.Indices, mut/signers/optional flags, and descriptions (None) align with expectations.
shank-idl/src/file.rs (1)
110-129: Correctly wires ShankAccounts into instruction IDL conversion.Early extraction and passing into
IdlInstructions::try_into_with_accountsis the right integration point.Please confirm there’s no double-iteration hazard on
ctx.structs()acrossinstructions()andaccounts()in large crates (should be fine since each creates a fresh iterator).shank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.json (1)
1-94: Fixture aligns with struct-based accounts expansion.Accounts, args, and discriminants look consistent; optionality encoded via
isOptional.Double-check naming conventions (e.g.,
systemProgramvs. Rustsystem_program) match the generator’s casing rules for this fixture comparison.shank-idl/tests/instructions.rs (3)
193-209: Good coverage: simple accounts struct round‑tripTest mirrors existing pattern and asserts full IDL equality. Looks solid.
211-226: Good: accounts struct with optional handlingConsistent with the fixture layout and ParseIdlConfig usage.
228-243: Good: complex struct scenario exercisedNice to see a complex coverage case added.
shank/tests/basic_context_test.rs (1)
1-17: LGTM: minimal mock and derive usageLocal AccountInfo mock and attributes look fine for exercising the macro.
shank/tests/expand/custom_lifetime.rs (1)
10-17: LGTM: lifetime variance coveredGood case ensuring lifetimes other than 'info are accepted.
shank/tests/simple_context_test.rs (2)
25-30: Strengthen assertions to validate optional flagSince the struct includes
#[account(optional)], assert the metadata reflects it.
[suggest_minor_issue]
Apply:let accounts = SimpleAccounts::__shank_accounts(); assert_eq!(accounts.len(), 3); +// payer, data, optional_account +assert_eq!(accounts[2].1, "optional_account"); +assert_eq!(accounts[2].5, true); // isOptional
13-23: LGTM: simple struct shape for __shank_accounts()Attributes and field order suit the metadata checks.
shank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.rs (1)
4-17: LGTM: fixture focuses on attributes, not typesAppropriate for AST-based IDL extraction; types being non-reference is fine here.
shank/tests/error_handling_test.rs (8)
10-21: LGTM: documents compile-time conflict clearlyGood to keep the conflicting case commented with rationale.
22-49: LGTM: signer vs optional_signer semantics validatedIndex mapping matches expanded tests tuple layout.
51-71: LGTM: description capture behaviorCovers presence/absence; concise.
73-98: LGTM: field order preservationValidates deterministic ordering.
100-172: LGTM: comprehensive flag matrixGood breadth across combinations including desc.
174-196: LGTM: lifetime-parameter variantsMatches expectations from expanded fixtures.
198-217: LGTM: special field names persisted verbatimCovers common edge cases.
219-245: LGTM: long description handlingValidates empty and long docs as well.
EXAMPLE_USAGE.md (1)
45-50: Remaining-accounts handling verified — no change requiredConfirmed: Context exposes pub remaining_accounts: &'a [U]; the macro builds it as &accounts[#expected_accounts..]; minimum-account validation returns Err(ProgramError::NotEnoughAccountKeys) when too few are provided; tests cover both extra and no-extra cases (shank/src/context.rs, shank-macro-impl/src/accounts/mod.rs, shank/tests/context_api_working_test.rs, shank/tests/context_usage_test.rs).
shank-macro-impl/src/accounts/account_field.rs (2)
60-66: Semantics: should optional_signer imply optional?Current metadata can mark optional_signer=true with optional=false. Consider setting optional=true when optional_signer is present, or document the intended semantics.
23-58: No action required — workspace uses syn 1.x (syn = 1.0.82)
Cargo.toml entries show syn = 1.0.82, so attr.parse_meta(), attr.path and NestedMeta are compatible; no migration to syn 2 required.shank-idl/ACCOUNTS_STRUCT_IDL.md (1)
41-46: Incorrect: placeholder-name mismatch claim — no change requiredBoth the IDL and the emitter already use "_accounts_struct{StructName}"; the "accountsStruct..." variant is not present in the repo. Found at shank-idl/ACCOUNTS_STRUCT_IDL.md:42 and shank-macro-impl/src/instruction/instruction.rs:155.
Likely an incorrect or invalid review comment.
shank-macro-impl/src/instruction/instruction.rs (1)
134-167: Unify placeholder naming and robustly extract struct nameUse the last path segment for the struct name (handles qualified paths) and standardize the placeholder to "_accounts_struct{StructName}" to align snapshots.
File: shank-macro-impl/src/instruction/instruction.rs
Lines: 134-167- Err(_) => { + Err(_) => { let accounts_source = AccountsSource::try_from(attrs)?; let accounts = match &accounts_source { AccountsSource::Inline(accs) => accs.0.clone(), AccountsSource::Struct(path) => { - // Create placeholder accounts based on the struct name - // The actual resolution will happen during IDL generation - // For now, create a single placeholder account that indicates this uses a struct - vec![InstructionAccount { - ident: ident.clone(), - index: Some(0), - name: format!("__accounts_struct_{}", path.get_ident().unwrap_or(&ident)), - writable: false, - signer: false, - optional_signer: false, - desc: Some(format!("Accounts defined by struct: {}", path.get_ident().unwrap_or(&ident))), - optional: false, - }] + // Placeholder resolved during IDL generation + let struct_name = path.segments.last() + .map(|seg| seg.ident.to_string()) + .unwrap_or_else(|| ident.to_string()); + vec![InstructionAccount { + ident: ident.clone(), + index: Some(0), + name: format!("__accounts_struct_{}", struct_name), + writable: false, + signer: false, + optional_signer: false, + desc: Some(format!("Accounts defined by struct: {}", struct_name)), + optional: false, + }] } }; let strategies: InstructionStrategies = attrs.into(); (accounts_source, accounts, strategies) }shank/tests/context_api_test.rs (1)
65-79: Great coverage on optional and mut/signer combos.These assertions give good signal on tuple ordering and flags for writable/signer/optional.
shank/tests/context_runtime_test.rs (2)
31-63: LGTM: feature-gated stubs make the tests portable.The cfg blocks keep tests compiling without solana-program; good separation.
153-173: Nice default-constraints sweep; consider asserting desc is None.You already assert None at index 6; good coverage.
shank-macro/src/lib.rs (2)
246-247: Approve — derive_instruction already accepts #[accounts(StructName)] and wires the extraction path.Parser and dispatch are implemented: shank-macro-impl/src/instruction/account_attrs.rs (is_accounts_struct_attr, from_accounts_struct_attr, try_from), wiring in shank-macro-impl/src/instruction/instruction.rs (uses to_accounts / AccountsSource), and account struct conversion in shank-macro-impl/src/accounts/account_struct.rs::to_instruction_accounts.
365-453: Approve — ShankAccounts macro emits IDL metadata and context helpers.
gen_impl defines pub fn __shank_accounts() -> Vec<(u32, &'static str, bool, bool, bool, bool, Option)> and a pub fn context(...) -> ::shank::Context<...> (see shank-macro-impl/src/accounts/mod.rs, around lines 61 and 111).shank-macro-impl/src/accounts/account_struct.rs (1)
25-35: Tuple order verified — no action required.
gen_account_metadata returns (index, name, writable, signer, optional_signer, optional, desc) in that exact order, matching gen_account_list usage.shank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.json (1)
41-49: Optional-signer semantics are implemented as-is; the suggested isOptional = optional || optional_signer is incorrect.shank-idl/src/accounts_extraction.rs:120–124 clears
optionalwhenoptional_signeris set, and shank-idl/src/idl_instruction.rs:358–362 mapsis_signer = signer,is_optional = optional,is_optional_signer = optional_signer.Likely an incorrect or invalid review comment.
shank/tests/final_context_test.rs (1)
52-53: Confirm Context generic usage matches the crate API.If Context is defined with lifetimes (e.g., Context<'info, T>), this may need to be Context<TestAccounts<'_>> or similar. Please verify and adjust.
shank-idl/src/idl_instruction.rs (1)
229-253: Incorrect — accounts_map is keyed by the short struct ident; last-segment lookup is correct.shank-idl/src/accounts_extraction.rs inserts keys with
let struct_name = struct_item.ident.to_string();followed byaccounts_map.insert(struct_name, accounts);(around lines 18–21), so resolving viastruct_path.segments.last()matches the map keys. No change required.Likely an incorrect or invalid review comment.
shank/tests/context_api_working_test.rs (2)
20-30: Good: ShankAccounts usage and field shapes look consistent.Derive, lifetimes, and Option<&AccountInfo> are coherent with the intended API.
96-101: Confirmed: optional-account sentinel is crate::ID.The generated context/macro explicitly maps optional fields to None when accounts[#idx].key == &crate::ID (see shank-render/src/context/render_context.rs:25 and shank-macro-impl/src/accounts/mod.rs:90-93, comment 108-111). The test's expectation that key == ID yields None is correct.
shank-idl/src/accounts_extraction.rs (1)
126-136: LGTM: InstructionAccount construction.Fields mapped coherently (index, ident, flags, desc).
shank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.rs (1)
1-44: LGTM: Struct-based accounts expansion setup.Accounts structs and enum wiring match the intended flow.
shank/tests/expand/complex_constraints.rs (1)
10-29: LGTM: Constraint coverage looks comprehensive.Good spread of mut, signer, optional, and combinations.
shank/tests/context_usage_test.rs (1)
25-33: API mismatch: tests call context(accounts, &program_id) and expect Result<...>.This relies on the macro fix in accounts/mod.rs. Without it, these tests won’t compile. Re-run after applying the macro patch.
Also applies to: 76-83, 150-158, 223-231, 271-275
shank/tests/context_issue_demo.rs (2)
25-28: API mismatch: context() currently returns Context, not Result.Once macro is fixed to return Result and take program_id, this test will be valid. Track dependency on macro change.
31-62: Ensure context() is cfg-gated off in non-spl builds.With the proposed macro change, context() won’t be generated here. This test should compile cleanly.
shank/tests/expand/complex_constraints.expanded.rs (1)
21-85: Expanded tuple shape/flags look correct.Writable/signer/optional_signer/optional flags and descriptions align with attributes.
Confirm expand golden files don’t expect context() to appear; if they do, update fixtures or ensure macro cfg-gates generation.
shank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.rs (3)
3-23: Struct fields use owned AccountInfo; OK for IDL, but not for context().This fixture is IDL-only. Ensure tests run without solana-program feature so context() isn’t generated; otherwise owned types will mismatch the macro’s reference-based context assignment.
25-41: Mixed instruction spec looks good.Combining #[accounts(ComplexAccounts)] with old-style inline accounts exercises the new expansion path well.
43-48: Local AccountInfo mock is fine for IDL.No changes needed.
shank/examples/context_usage_example.rs (1)
23-36: Docs vs behavior: optional accounts must use Option.You did this correctly for metadata. Good.
Please confirm the generated ctx.metadata type is Option<&AccountInfo<'_>> as intended across all derives.
shank-macro-impl/src/instruction/account_attrs.rs (1)
65-153: Inline account parser looks solid; nice coverage of aliases and mutual exclusivity.No functional issues found.
shank/examples/anchor_style_example.rs (1)
112-127: Example text aligns with Option usage.Messages correctly describe Option<&AccountInfo>. After fixing types above, this stays consistent.
shank/examples/full_solana_example.rs (1)
79-109: Processor paths look consistent with context(program_id, accounts).Logging keys and optional handling are clear.
Please confirm Pubkey implements Display in your MSRV; if not, switch to {:?}.
Also applies to: 111-129
shank/tests/enhanced_accounts_test.rs (1)
92-99: Mixed old/new styles exercised correctly.Good coverage ensuring backward compatibility.
Also applies to: 114-123
shank/examples/accounts_struct_example.rs (1)
11-36: LGTM: clear, idiomatic use of ShankAccounts for examplesField attributes (mut/signer/optional/desc) look correct, and using PhantomData keeps this example buildable without Solana deps. No functional concerns.
Also applies to: 38-51, 53-66
shank/tests/instruction_integration_test.rs (1)
77-79: Note on optional fields in testsUsing #[account(optional)] with a non-Option field is fine here because you’re only asserting IDL metadata via __shank_accounts(). If you later exercise context() runtime generation, these should be Option<AccountInfo<'info>> (or similar) to reflect optionality.
Also applies to: 159-161
| // Program ID - normally created by declare_id! macro | ||
| pub const ID: [u8; 32] = [1; 32]; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Program ID type should reflect Solana’s Pubkey/declare_id!
Using [u8; 32] for ID can mislead users. Prefer declare_id! so examples align with real projects.
Apply:
-// Program ID - normally created by declare_id! macro
-pub const ID: [u8; 32] = [1; 32];
+// Program ID - typically created by `declare_id!`, which defines `crate::ID: Pubkey`
+use solana_program::{declare_id, pubkey::Pubkey};
+declare_id!("11111111111111111111111111111111");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Program ID - normally created by declare_id! macro | |
| pub const ID: [u8; 32] = [1; 32]; | |
| // Program ID - typically created by `declare_id!`, which defines `crate::ID: Pubkey` | |
| use solana_program::{declare_id, pubkey::Pubkey}; | |
| declare_id!("11111111111111111111111111111111"); |
🤖 Prompt for AI Agents
In EXAMPLE_USAGE.md around lines 10 to 12, replace the raw byte array pub const
ID: [u8; 32] = [1; 32]; with the standard Solana pattern using the declare_id!
macro so examples match real projects: import or reference
solana_program::declare_id! (or the appropriate crate path in the example) and
use declare_id!("ReplaceWithYourProgramIdHere"); ensuring the example shows the
macro usage rather than a [u8; 32] literal and updating any surrounding text to
mention inserting the real program id string.
| ```rust | ||
| // When optional account is provided: | ||
| let accounts = [payer, vault, authority_account, system_program]; | ||
| let ctx = CreateVaultAccounts::context(&accounts); | ||
| assert!(ctx.accounts.optional_authority.is_some()); | ||
|
|
||
| // When optional account is NOT provided (pass program ID as placeholder): | ||
| let accounts = [payer, vault, program_id_placeholder, system_program]; | ||
| let ctx = CreateVaultAccounts::context(&accounts); | ||
| assert!(ctx.accounts.optional_authority.is_none()); | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
Clarify the “program ID placeholder”
Explicitly note that the placeholder must be an AccountInfo whose key equals crate::ID; otherwise readers may try to pass a Pubkey.
Apply:
-// When optional account is NOT provided (pass program ID as placeholder):
-let accounts = [payer, vault, program_id_placeholder, system_program];
+// When optional account is NOT provided (pass an AccountInfo whose key == crate::ID as placeholder):
+// e.g., let program_id_placeholder = AccountInfo { key: &ID.to_bytes(), data: &[], owner: &ID.to_bytes() /* mock */ };
+let accounts = [payer, vault, program_id_placeholder /* AccountInfo with key == crate::ID */, system_program];Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In EXAMPLE_USAGE.md around lines 31 to 41, the comment about passing a "program
ID placeholder" is unclear; update the example text to explicitly state that the
placeholder must be an AccountInfo whose key equals crate::ID (not a Pubkey),
and instruct readers to construct or pass an AccountInfo with key == crate::ID
when they want to omit the optional account so the context check behaves as
shown.
| 1. **Ergonomic API**: No need to pass program ID parameter - uses `crate::ID` automatically | ||
| 2. **Type Safety**: Optional accounts use `Option<&AccountInfo>` types | ||
| 3. **Runtime Detection**: Checks if `account.key == crate::ID` to determine None/Some | ||
| 4. **IDL Generation**: Proper `"isOptional": true` flags in generated IDL | ||
| 5. **Remaining Accounts**: Automatically handles extra accounts beyond the struct definition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
🧩 Analysis chain
Inconsistent API description: context() with or without program_id
Elsewhere (examples/… and docs), context(program_id, accounts) appears. Here it claims no program_id is needed. Please unify and document the canonical signature and when each form applies.
Would you confirm the intended API and update all docs accordingly?
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "--- repo root listing ---"
ls -la
echo
echo "---- EXAMPLE_USAGE.md (first 200 lines) ----"
if [ -f EXAMPLE_USAGE.md ]; then
sed -n '1,200p' EXAMPLE_USAGE.md || true
else
echo "EXAMPLE_USAGE.md not found at repo root"
fi
echo
echo "---- rg: search for 'context(' across repo ----"
rg -n -S -C2 "context\(" || true
echo
echo "---- rg: search for 'context(program_id' ----"
rg -n -S -C2 "context\(.*program_id" || true
echo
echo "---- rg: search for the exact substring 'context(program_id, accounts' ----"
rg -n -F "context(program_id, accounts" || true
echo
echo "---- rg: look for function/definition 'fn context' / 'def context' / 'function context' ----"
rg -n -S -C2 "fn context\b" || true
rg -n -S -C2 "\bdef context\b" || true
rg -n -S -C2 "\bfunction context\b" || trueLength of output: 28416
Unify the context(...) API to a single canonical signature and apply it across the repo.
Repo mixes context(&[AccountInfo]) and context(program_id, accounts) in docs, examples, tests, and generated code — this must be unified.
Files to update:
- Implementations / generator: shank-macro-impl/src/accounts/mod.rs, shank-render/src/context/render_context.rs
- Macro docs: shank-macro/src/lib.rs
- Examples / docs: EXAMPLE_USAGE.md, shank/examples/full_solana_example.rs, shank/examples/context_usage_example.rs, shank/examples/anchor_style_example.rs, shank/examples/accounts_struct_example.rs
- Tests: shank/tests/* (e.g. context_usage_test.rs, final_context_test.rs, context_issue_demo.rs)
Action required: choose the canonical signature (either accounts-only — using crate::ID as the placeholder for missing optional accounts — or explicit program_id + accounts), update codegen/macros, fix all examples/tests/docs to match, and document the chosen behavior for optional accounts in EXAMPLE_USAGE.md and macro docs.
| } | ||
| ``` | ||
|
|
||
| This follows Solana's modern optional accounts pattern where missing optional accounts are represented by the program ID. No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
Scope the claim about optional-accounts convention
“Solana’s modern optional accounts pattern” is framework-convention, not a protocol rule. Rephrase to avoid overstating.
Apply:
-This follows Solana's modern optional accounts pattern where missing optional accounts are represented by the program ID.
+This follows a common framework convention where missing optional accounts are represented by the program ID.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| This follows Solana's modern optional accounts pattern where missing optional accounts are represented by the program ID. | |
| This follows a common framework convention where missing optional accounts are represented by the program ID. |
🤖 Prompt for AI Agents
In EXAMPLE_USAGE.md around line 64, the sentence asserts "Solana’s modern
optional accounts pattern" as a protocol rule; rephrase to scope it as a
framework/convention instead of a protocol requirement. Update the wording to
indicate this is a common framework or developer convention (e.g., "a common
convention in many Solana frameworks") and avoid language that implies it is
mandated by the Solana protocol.
| #[account(mut, signer, desc = "The payer and authority")] | ||
| pub payer: AccountInfo<'info>, | ||
|
|
||
| #[account(mut, desc = "The vault account to create")] | ||
| pub vault: AccountInfo<'info>, | ||
|
|
||
| #[account(desc = "System program")] | ||
| pub system_program: AccountInfo<'info>, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
Use &'info AccountInfo<'info> in examples for consistency
Examples elsewhere (tests/examples) use references. Update this snippet to avoid confusion.
- pub payer: AccountInfo<'info>,
+ pub payer: &'info AccountInfo<'info>,
- pub vault: AccountInfo<'info>,
+ pub vault: &'info AccountInfo<'info>,
- pub system_program: AccountInfo<'info>,
+ pub system_program: &'info AccountInfo<'info>,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[account(mut, signer, desc = "The payer and authority")] | |
| pub payer: AccountInfo<'info>, | |
| #[account(mut, desc = "The vault account to create")] | |
| pub vault: AccountInfo<'info>, | |
| #[account(desc = "System program")] | |
| pub system_program: AccountInfo<'info>, | |
| } | |
| #[account(mut, signer, desc = "The payer and authority")] | |
| pub payer: &'info AccountInfo<'info>, | |
| #[account(mut, desc = "The vault account to create")] | |
| pub vault: &'info AccountInfo<'info>, | |
| #[account(desc = "System program")] | |
| pub system_program: &'info AccountInfo<'info>, | |
| } |
🤖 Prompt for AI Agents
In shank-idl/ACCOUNTS_STRUCT_IDL.md around lines 16 to 24, the account fields
are declared as owned AccountInfo<'info> which is inconsistent with other
examples; update the declarations to use references by changing each field to
&'info AccountInfo<'info> (preserving attributes like mut and signer where
present) so the snippet matches the rest of the codebase and avoids confusion.
| pub optional_authority: Option<&'info AccountInfo<'info>>, | ||
| } | ||
|
|
||
| fn main() {} No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
Add trailing newline
Minor formatting polish for editors and linters.
🤖 Prompt for AI Agents
In shank/tests/expand/optional_accounts.rs around line 22, the file is missing a
trailing newline; add a newline at the end of the file so the final line ends
with a newline character to satisfy editors and linters.
| #[test] | ||
| fn test_idl_generation_basic_accounts() { | ||
| #[derive(ShankAccounts)] | ||
| pub struct BasicAccounts<'info> { | ||
| #[account(mut, signer, desc = "The payer account")] | ||
| pub payer: &'info AccountInfo<'info>, | ||
|
|
||
| #[account(mut, desc = "The data account")] | ||
| pub data: &'info AccountInfo<'info>, | ||
|
|
||
| #[account(desc = "The system program")] | ||
| pub system_program: &'info AccountInfo<'info>, | ||
| } | ||
|
|
||
| let idl = BasicAccounts::__shank_accounts(); | ||
|
|
||
| // Verify we have 3 accounts | ||
| assert_eq!(idl.len(), 3); | ||
|
|
||
| // Check payer account | ||
| assert_eq!(idl[0].0, 0); // index | ||
| assert_eq!(idl[0].1, "payer"); // name | ||
| assert_eq!(idl[0].2, true); // mut | ||
| assert_eq!(idl[0].3, true); // signer | ||
| assert_eq!(idl[0].4, false); // optional_signer (false for regular signer) | ||
| assert_eq!(idl[0].5, false); // optional | ||
| assert_eq!(idl[0].6, Some("The payer account".to_string())); // description | ||
|
|
||
| // Check data account | ||
| assert_eq!(idl[1].0, 1); | ||
| assert_eq!(idl[1].1, "data"); | ||
| assert_eq!(idl[1].2, true); // mut | ||
| assert_eq!(idl[1].3, false); // not signer | ||
| assert_eq!(idl[1].4, false); // optional_signer | ||
| assert_eq!(idl[1].5, false); // not optional | ||
| assert_eq!(idl[1].6, Some("The data account".to_string())); | ||
|
|
||
| // Check system program | ||
| assert_eq!(idl[2].0, 2); | ||
| assert_eq!(idl[2].1, "system_program"); | ||
| assert_eq!(idl[2].2, false); // not mut | ||
| assert_eq!(idl[2].3, false); // not signer | ||
| assert_eq!(idl[2].4, false); // optional_signer | ||
| assert_eq!(idl[2].5, false); // not optional | ||
| assert_eq!(idl[2].6, Some("The system program".to_string())); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
Solid coverage of tuple semantics and flag combinations.
Tests accurately validate indices, names, mut/signers, optional flags, and descriptions across scenarios.
Consider a small helper or a type alias to avoid tuple indexing noise in assertions for readability:
type AccountMeta = (u32, &'static str, bool, bool, bool, bool, Option<String>);
Also applies to: 57-90, 92-160, 162-181, 183-203, 205-212, 214-231, 233-263, 265-290
🤖 Prompt for AI Agents
In shank/tests/idl_generation_test.rs around lines 10 to 55, the assertions use
raw tuple indexing which harms readability; add a local type alias like
AccountMeta = (u32, &'static str, bool, bool, bool, bool, Option<String>) near
the top of the test file and then replace the index-based assertions with either
pattern destructuring (let (idx, name, is_mut, is_signer, opt_signer, opt, desc)
= idl[n]; assert_eq!(idx, ...); ...) or by binding idl[n] to a variable of type
AccountMeta before asserting; apply the same alias/destructuring approach to the
other listed test ranges to reduce tuple-index noise and make assertions
clearer.
| #[account(optional_signer, desc = "Authority on the vault")] | ||
| pub authority: AccountInfo<'info>, | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional signer field should use Option and standard flags.
Type isn’t optional though attribute says optional_signer. Prefer Option<...> with #[account(optional, signer)] for consistency with other tests and API.
- #[account(optional_signer, desc = "Authority on the vault")]
- pub authority: AccountInfo<'info>,
+ #[account(optional, signer, desc = "Authority on the vault")]
+ pub authority: Option<AccountInfo<'info>>,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[account(optional_signer, desc = "Authority on the vault")] | |
| pub authority: AccountInfo<'info>, | |
| #[account(optional, signer, desc = "Authority on the vault")] | |
| pub authority: Option<AccountInfo<'info>>, |
🤖 Prompt for AI Agents
In shank/tests/instruction_with_accounts_struct_test.rs around lines 25 to 27,
the field is marked optional_signer but is typed as AccountInfo<'info>; change
the field type to Option<AccountInfo<'info>> and replace the attribute with the
standard #[account(optional, signer)] form, and update any test setup/uses to
pass Some(account) or None and handle the Option (e.g., unwrap or map) where the
field is accessed.
| fn main() { | ||
| println!("Simple context test"); | ||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove main() from integration test
Avoid defining fn main() in tests.
Apply:
-fn main() {
- println!("Simple context test");
-}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn main() { | |
| println!("Simple context test"); | |
| } |
🤖 Prompt for AI Agents
In shank/tests/simple_context_test.rs around lines 32 to 34, the file currently
defines a standalone fn main() in an integration test which should be removed;
replace the top-level main function with one or more #[test] functions (or move
helper code to a module and expose test functions) so the test runner executes
them — delete fn main(), add a #[test] fn simple_context_test() { /* move test
logic here and use assertions instead of println! */ } and ensure the file
compiles as an integration test without a main.
| fn main() { | ||
| println!("Working context demo"); | ||
| println!("The context() method should be available on DemoAccounts"); | ||
| println!("It should return Context<DemoAccounts> with accounts and remaining_accounts fields"); | ||
|
|
||
| // For now, let's just test that the IDL generation works | ||
| let accounts_idl = DemoAccounts::__shank_accounts(); | ||
| println!("Generated {} accounts in IDL", accounts_idl.len()); | ||
|
|
||
| // In a real program, this would work: | ||
| // let ctx = DemoAccounts::context(&accounts); | ||
| // ctx.accounts.payer.key // access payer | ||
| // ctx.remaining_accounts // access any extra accounts | ||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
main() in an integration test will conflict with the test harness.
Integration tests in tests/ are compiled with a generated main; defining your own main leads to build failure. Convert to a #[test] or move to examples/.
-fn main() {
+#[test]
+fn working_context_demo() {
println!("Working context demo");
println!("The context() method should be available on DemoAccounts");
println!("It should return Context<DemoAccounts> with accounts and remaining_accounts fields");
// For now, let's just test that the IDL generation works
let accounts_idl = DemoAccounts::__shank_accounts();
println!("Generated {} accounts in IDL", accounts_idl.len());
// In a real program, this would work:
// let ctx = DemoAccounts::context(&accounts);
// ctx.accounts.payer.key // access payer
// ctx.remaining_accounts // access any extra accounts
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn main() { | |
| println!("Working context demo"); | |
| println!("The context() method should be available on DemoAccounts"); | |
| println!("It should return Context<DemoAccounts> with accounts and remaining_accounts fields"); | |
| // For now, let's just test that the IDL generation works | |
| let accounts_idl = DemoAccounts::__shank_accounts(); | |
| println!("Generated {} accounts in IDL", accounts_idl.len()); | |
| // In a real program, this would work: | |
| // let ctx = DemoAccounts::context(&accounts); | |
| // ctx.accounts.payer.key // access payer | |
| // ctx.remaining_accounts // access any extra accounts | |
| } | |
| #[test] | |
| fn working_context_demo() { | |
| println!("Working context demo"); | |
| println!("The context() method should be available on DemoAccounts"); | |
| println!("It should return Context<DemoAccounts> with accounts and remaining_accounts fields"); | |
| // For now, let's just test that the IDL generation works | |
| let accounts_idl = DemoAccounts::__shank_accounts(); | |
| println!("Generated {} accounts in IDL", accounts_idl.len()); | |
| // In a real program, this would work: | |
| // let ctx = DemoAccounts::context(&accounts); | |
| // ctx.accounts.payer.key // access payer | |
| // ctx.remaining_accounts // access any extra accounts | |
| } |
🤖 Prompt for AI Agents
In shank/tests/working_context_demo.rs around lines 22 to 35, the file defines a
main() which conflicts with the test harness for integration tests; replace main
with a test or move the file to examples. To fix: either convert the function to
a #[test] (rename to e.g. fn working_context_demo_test) and change prints to
assertions (e.g. assert!(DemoAccounts::__shank_accounts().len() > 0) or other
meaningful asserts), or move the file out of tests/ into examples/ (keeping
main) so it’s built as a binary example instead of an integration test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review continued from previous batch...
| /// Extract ShankAccounts structs and their metadata | ||
| pub fn extract_shank_accounts_structs<'a>( | ||
| structs: impl Iterator<Item = &'a ItemStruct>, | ||
| ) -> Result<HashMap<String, Vec<InstructionAccount>>> { | ||
| let mut accounts_map = HashMap::new(); | ||
|
|
||
| for struct_item in structs { | ||
| if let Some(_attr) = get_derive_attr(&struct_item.attrs, DERIVE_ACCOUNTS_ATTR) { | ||
| let struct_name = struct_item.ident.to_string(); | ||
| let accounts = extract_accounts_from_struct(struct_item)?; | ||
| accounts_map.insert(struct_name, accounts); | ||
| } | ||
| } | ||
|
|
||
| Ok(accounts_map) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
Naming mismatch with guidelines (ShankAccount vs ShankAccounts).
The code targets ShankAccounts; update docs/guidelines to avoid confusion or provide a compatibility note.
🤖 Prompt for AI Agents
In shank-idl/src/accounts_extraction.rs around lines 11 to 26, the doc comment
and naming refer to "ShankAccounts" but project guidelines use "ShankAccount"
(singular) causing mismatch; update the comment and any exposed identifiers to
match the canonical name: change the docstring to "Extract ShankAccount structs
and their metadata" (or add a brief compatibility note if keeping the plural),
and update project docs/guidelines to reflect the chosen singular/plural
convention so names are consistent across code and docs.
| /// Extract individual accounts from a ShankAccounts struct by calling its __shank_accounts method | ||
| fn extract_accounts_from_struct(struct_item: &ItemStruct) -> Result<Vec<InstructionAccount>> { | ||
| // This is where we need to get the account metadata. | ||
| // The challenge is that at parse time, we can't execute the __shank_accounts() method. | ||
| // We need to parse the struct fields and their #[account(...)] attributes directly. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
Fix doc comment to reflect actual behavior.
This function parses field attributes; it does not call __shank_accounts().
-/// Extract individual accounts from a ShankAccounts struct by calling its __shank_accounts method
+/// Extract individual accounts from a ShankAccounts struct by parsing its #[account(...)] field attributes📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Extract individual accounts from a ShankAccounts struct by calling its __shank_accounts method | |
| fn extract_accounts_from_struct(struct_item: &ItemStruct) -> Result<Vec<InstructionAccount>> { | |
| // This is where we need to get the account metadata. | |
| // The challenge is that at parse time, we can't execute the __shank_accounts() method. | |
| // We need to parse the struct fields and their #[account(...)] attributes directly. | |
| /// Extract individual accounts from a ShankAccounts struct by parsing its #[account(...)] field attributes | |
| fn extract_accounts_from_struct(struct_item: &ItemStruct) -> Result<Vec<InstructionAccount>> { | |
| // This is where we need to get the account metadata. | |
| // The challenge is that at parse time, we can't execute the __shank_accounts() method. | |
| // We need to parse the struct fields and their #[account(...)] attributes directly. | |
🤖 Prompt for AI Agents
In shank-idl/src/accounts_extraction.rs around lines 28 to 33, the doc comment
incorrectly states that the function extracts accounts by calling
__shank_accounts(); update the comment to accurately describe current behavior:
explain that the function parses the struct fields and reads their
#[account(...)] attributes at parse time (since __shank_accounts() cannot be
executed), and mention it returns a Vec<InstructionAccount> derived from those
attributes. Keep the comment concise and aligned with the implemented parsing
behavior.
| let has_option_type = if let Type::Path(type_path) = &field.ty { | ||
| if let Some(segment) = type_path.path.segments.first() { | ||
| segment.ident == "Option" | ||
| } else { | ||
| false | ||
| } | ||
| } else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Option detection is fragile (uses first segment).
Fully-qualified types (e.g., std::option::Option) won’t be detected. Use the last path segment.
- let has_option_type = if let Type::Path(type_path) = &field.ty {
- if let Some(segment) = type_path.path.segments.first() {
- segment.ident == "Option"
- } else {
- false
- }
- } else {
- false
- };
+ let has_option_type = matches!(&field.ty, Type::Path(type_path)
+ if type_path.path.segments.last().map_or(false, |seg| seg.ident == "Option"));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let has_option_type = if let Type::Path(type_path) = &field.ty { | |
| if let Some(segment) = type_path.path.segments.first() { | |
| segment.ident == "Option" | |
| } else { | |
| false | |
| } | |
| } else { | |
| let has_option_type = matches!(&field.ty, Type::Path(type_path) | |
| if type_path.path.segments.last().map_or(false, |seg| seg.ident == "Option")); |
🤖 Prompt for AI Agents
In shank-idl/src/accounts_extraction.rs around lines 70 to 76, the current
Option<T> detection inspects the first path segment which misses fully-qualified
paths like std::option::Option; change the logic to inspect the last path
segment instead (e.g., use path.segments.last()) and compare that segment.ident
to "Option", keeping the existing Type::Path check; ensure you handle the Option
being present or not (map/and_then or pattern) so you don’t panic on empty
segments.
| // Parse #[account(...)] attributes | ||
| for attr in &field.attrs { | ||
| if attr.path.is_ident("account") { | ||
| // Use a simple string-based parsing approach for now | ||
| // This is a simplified version - in production we'd want more robust parsing | ||
| let tokens_str = attr.tokens.to_string(); | ||
|
|
||
| // Simple parsing of common attributes | ||
| if tokens_str.contains("mut") || tokens_str.contains("writable") { | ||
| writable = true; | ||
| } | ||
| if tokens_str.contains("signer") { | ||
| signer = true; | ||
| } | ||
| if tokens_str.contains("optional_signer") { | ||
| optional_signer = true; | ||
| } else if tokens_str.contains("optional") { | ||
| optional = true; | ||
| } | ||
|
|
||
| // Extract description using simple regex-like approach | ||
| if let Some(desc_start) = tokens_str.find("desc = \"") { | ||
| let desc_content = &tokens_str[desc_start + 8..]; | ||
| if let Some(desc_end) = desc_content.find('"') { | ||
| desc = Some(desc_content[..desc_end].to_string()); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
🧩 Analysis chain
Replace string-based parsing with syn’s nested meta; support alias attributes.
String contains() is brittle (spacing/ordering) and misses aliases (write/writ/w, sign/sig/s, opt/option). Parse tokens robustly and accept aliases to match the macro’s surface.
- // Parse #[account(...)] attributes
- for attr in &field.attrs {
- if attr.path.is_ident("account") {
- // Use a simple string-based parsing approach for now
- // This is a simplified version - in production we'd want more robust parsing
- let tokens_str = attr.tokens.to_string();
-
- // Simple parsing of common attributes
- if tokens_str.contains("mut") || tokens_str.contains("writable") {
- writable = true;
- }
- if tokens_str.contains("signer") {
- signer = true;
- }
- if tokens_str.contains("optional_signer") {
- optional_signer = true;
- } else if tokens_str.contains("optional") {
- optional = true;
- }
-
- // Extract description using simple regex-like approach
- if let Some(desc_start) = tokens_str.find("desc = \"") {
- let desc_content = &tokens_str[desc_start + 8..];
- if let Some(desc_end) = desc_content.find('"') {
- desc = Some(desc_content[..desc_end].to_string());
- }
- }
- }
- }
+ // Parse #[account(...)] attributes robustly and support aliases
+ for attr in &field.attrs {
+ #[allow(deprecated)]
+ if attr.path().is_ident("account") {
+ attr.parse_nested_meta(|meta| {
+ let key = meta.path.segments.last().map(|s| s.ident.to_string()).unwrap_or_default();
+ match key.as_str() {
+ // writable variants
+ "mut" | "writable" | "write" | "writ" | "w" => writable = true,
+ // signer variants
+ "signer" | "sign" | "sig" | "s" => signer = true,
+ // optional variants
+ "optional_signer" => optional_signer = true,
+ "optional" | "opt" | "option" => optional = true,
+ // desc = "..."
+ "desc" => {
+ let val: shank_macro_impl::syn::LitStr = meta.value()?.parse()?;
+ desc = Some(val.value());
+ }
+ _ => {}
+ }
+ Ok(())
+ })?;
+ }
+ }🏁 Script executed:
#!/bin/bash
# Ensure syn 2.x is in use (parse_nested_meta API)
rg -n --glob '**/Cargo.toml' -C2 'syn\s*='
# Grep for alias attributes in tests to confirm coverage alignment
rg -n --type=rust -C2 '#\[account\((write|writ|w|sign|sig|s|opt|option)\b'Length of output: 4788
Use Attribute::parse_nested_meta for robust #[account(...)] parsing with alias support
- In shank-idl/src/accounts_extraction.rs, replace the
tokens_str.contains(...)logic withattr.parse_nested_meta(|meta| { … }), matching onmeta.pathvariants:
• writable:"mut"|"writable"|"write"|"writ"|"w"
• signer:"signer"|"sign"|"sig"|"s"
• optional:"optional_signer"; and"optional"|"opt"|"option" - Parse
descviameta.value()?.parse::<LitStr>()?instead of manual string slicing (requiresfullfeature forparse_nested_meta) (docs.rs) - Remove brittle, order- and spacing-dependent string parsing entirely.
🤖 Prompt for AI Agents
In shank-idl/src/accounts_extraction.rs around lines 80 to 107, replace the
brittle tokens_str.contains(...) string parsing with syn's
Attribute::parse_nested_meta to robustly parse #[account(...)] entries: call
attr.parse_nested_meta(|meta| { ... }) (requires syn "full" feature), match on
meta.path to set writable when the path is any of
"mut","writable","write","writ","w", set signer when path is
"signer","sign","sig","s", set optional_signer for "optional_signer" and
optional for "optional","opt","option"; for desc extract the literal via
meta.value()?.parse::<LitStr>()? instead of manual slicing; remove the old
string-based checks and ensure error propagation or sensible defaults on parse
failures.
| ensure!( | ||
| discriminant < u8::MAX as usize, | ||
| anyhow!( | ||
| "Instruction variant discriminants have to be <= u8::MAX ({}), \ | ||
| but the discriminant of variant '{}' is {}", | ||
| u8::MAX, | ||
| ident, | ||
| discriminant | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Off-by-one on discriminant bound (excludes 255).
The check uses < u8::MAX but the error text says <=. Use <= to accept 255.
- ensure!(
- discriminant < u8::MAX as usize,
+ ensure!(
+ discriminant <= u8::MAX as usize,
anyhow!(
"Instruction variant discriminants have to be <= u8::MAX ({}), \
but the discriminant of variant '{}' is {}",
u8::MAX,
ident,
discriminant
)
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ensure!( | |
| discriminant < u8::MAX as usize, | |
| anyhow!( | |
| "Instruction variant discriminants have to be <= u8::MAX ({}), \ | |
| but the discriminant of variant '{}' is {}", | |
| u8::MAX, | |
| ident, | |
| discriminant | |
| ) | |
| ); | |
| ensure!( | |
| discriminant <= u8::MAX as usize, | |
| anyhow!( | |
| "Instruction variant discriminants have to be <= u8::MAX ({}), \ | |
| but the discriminant of variant '{}' is {}", | |
| u8::MAX, | |
| ident, | |
| discriminant | |
| ) | |
| ); |
🤖 Prompt for AI Agents
In shank-idl/src/idl_instruction.rs around lines 147 to 156, the discriminant
bound check is off-by-one: it currently uses `< u8::MAX as usize` which rejects
the valid value 255 while the message states "<=". Change the comparison to `<=
u8::MAX as usize` so discriminant 255 is accepted, and keep the error message
as-is; ensure the types remain consistent (usize vs u8::MAX) when making the
comparison.
| @@ -0,0 +1,43 @@ | |||
| use shank::ShankAccounts; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
Unused import.
Same here; remove unused import.
-use shank::ShankAccounts;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use shank::ShankAccounts; |
🤖 Prompt for AI Agents
In shank/tests/expand/custom_lifetime.expanded.rs around line 1, the statement
"use shank::ShankAccounts;" is an unused import; remove that import line from
the file (or replace it with a used import if intended) and run the build/test
to ensure no other references rely on it.
| @@ -0,0 +1,54 @@ | |||
| use shank::ShankAccounts; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
Unused import.
use shank::ShankAccounts; isn’t needed in the expanded output; removing avoids warnings.
-use shank::ShankAccounts;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use shank::ShankAccounts; |
🤖 Prompt for AI Agents
In shank/tests/expand/optional_accounts.expanded.rs at lines 1-1, the line `use
shank::ShankAccounts;` is an unused import; remove this use statement from the
expanded file (or the generated source) so the compiler no longer emits an
unused import warning, and re-run the build/generation to confirm no other
references to ShankAccounts remain.
| pub struct AccountInfo<'info> { | ||
| pub key: &'info [u8; 32], | ||
| pub data: &'info [u8], | ||
| pub owner: &'info [u8; 32], | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick
Provide key() like other tests so optional detection can work.
Other tests implement key() on AccountInfo; add the same here for consistency.
pub struct AccountInfo<'info> {
pub key: &'info [u8; 32],
pub data: &'info [u8],
pub owner: &'info [u8; 32],
}
+
+impl<'info> AccountInfo<'info> {
+ pub fn key(&self) -> &'info [u8; 32] {
+ self.key
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub struct AccountInfo<'info> { | |
| pub key: &'info [u8; 32], | |
| pub data: &'info [u8], | |
| pub owner: &'info [u8; 32], | |
| } | |
| pub struct AccountInfo<'info> { | |
| pub key: &'info [u8; 32], | |
| pub data: &'info [u8], | |
| pub owner: &'info [u8; 32], | |
| } | |
| impl<'info> AccountInfo<'info> { | |
| pub fn key(&self) -> &'info [u8; 32] { | |
| self.key | |
| } | |
| } |
🤖 Prompt for AI Agents
In shank/tests/final_context_test.rs around lines 4 to 8, AccountInfo is missing
the key() accessor used by other tests; add an impl block that defines a
key(&self) -> &'info [u8; 32] method that returns self.key so optional account
detection works the same as the other tests.
| #[test] | ||
| fn test_minimal_accounts() { | ||
| let payer_key = [1u8; 32]; | ||
| let data_key = [2u8; 32]; | ||
|
|
||
| let payer = AccountInfo { | ||
| key: &payer_key, | ||
| data: &[], | ||
| owner: &[0; 32], | ||
| }; | ||
|
|
||
| let data = AccountInfo { | ||
| key: &data_key, | ||
| data: &[], | ||
| owner: &[0; 32], | ||
| }; | ||
|
|
||
| // Only provide required accounts (payer and data, no optional) | ||
| let accounts = [payer, data]; | ||
|
|
||
| let ctx: Context<TestAccounts> = TestAccounts::context(&accounts); | ||
|
|
||
| assert_eq!(ctx.accounts.payer.key, &payer_key); | ||
| assert_eq!(ctx.accounts.data.key, &data_key); | ||
| assert!(ctx.accounts.optional_account.is_some()); // Still present at index 2, but would be None if we had proper ID detection | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix optional-account semantics in the minimal test (should be None, not Some).
When only required accounts are supplied, the optional account must be None. Either:
- pass a third placeholder with key == ID, or
- allow shorter slices and treat missing tail slots as None.
Apply this to use the ID sentinel and assert None:
@@
- // Only provide required accounts (payer and data, no optional)
- let accounts = [payer, data];
+ // Provide a placeholder for the optional account using the sentinel ID
+ let optional_placeholder = AccountInfo {
+ key: &ID,
+ data: &[],
+ owner: &[0; 32],
+ };
+ let accounts = [payer, data, optional_placeholder];
@@
- assert!(ctx.accounts.optional_account.is_some()); // Still present at index 2, but would be None if we had proper ID detection
+ assert!(ctx.accounts.optional_account.is_none());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[test] | |
| fn test_minimal_accounts() { | |
| let payer_key = [1u8; 32]; | |
| let data_key = [2u8; 32]; | |
| let payer = AccountInfo { | |
| key: &payer_key, | |
| data: &[], | |
| owner: &[0; 32], | |
| }; | |
| let data = AccountInfo { | |
| key: &data_key, | |
| data: &[], | |
| owner: &[0; 32], | |
| }; | |
| // Only provide required accounts (payer and data, no optional) | |
| let accounts = [payer, data]; | |
| let ctx: Context<TestAccounts> = TestAccounts::context(&accounts); | |
| assert_eq!(ctx.accounts.payer.key, &payer_key); | |
| assert_eq!(ctx.accounts.data.key, &data_key); | |
| assert!(ctx.accounts.optional_account.is_some()); // Still present at index 2, but would be None if we had proper ID detection | |
| } | |
| #[test] | |
| fn test_minimal_accounts() { | |
| let payer_key = [1u8; 32]; | |
| let data_key = [2u8; 32]; | |
| let payer = AccountInfo { | |
| key: &payer_key, | |
| data: &[], | |
| owner: &[0; 32], | |
| }; | |
| let data = AccountInfo { | |
| key: &data_key, | |
| data: &[], | |
| owner: &[0; 32], | |
| }; | |
| // Provide a placeholder for the optional account using the sentinel ID | |
| let optional_placeholder = AccountInfo { | |
| key: &ID, | |
| data: &[], | |
| owner: &[0; 32], | |
| }; | |
| let accounts = [payer, data, optional_placeholder]; | |
| let ctx: Context<TestAccounts> = TestAccounts::context(&accounts); | |
| assert_eq!(ctx.accounts.payer.key, &payer_key); | |
| assert_eq!(ctx.accounts.data.key, &data_key); | |
| assert!(ctx.accounts.optional_account.is_none()); | |
| } |
🤖 Prompt for AI Agents
In shank/tests/final_context_test.rs around lines 61-86, the minimal-accounts
test currently treats the optional account as Some because the accounts array
length provides that slot; to fix, supply the optional-account sentinel ID so
the framework recognizes it as missing and the field becomes None: add a third
AccountInfo with key set to the library's ID sentinel (e.g. the constant ID or
&[0u8;32] if that is the sentinel) and matching placeholder data/owner, then
update the assertion to assert!(ctx.accounts.optional_account.is_none());
alternatively, if the project prefers slice-shortening behavior, adjust
TestAccounts::context to accept shorter slices and map missing tail slots to
None instead (but prefer adding the sentinel in this test).
| fn main() { | ||
| println!("Final context test - context method is working!"); | ||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove main() from integration test.
Integration tests are built with the test harness main; providing your own main can conflict.
-fn main() {
- println!("Final context test - context method is working!");
-}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn main() { | |
| println!("Final context test - context method is working!"); | |
| } |
🤖 Prompt for AI Agents
In shank/tests/final_context_test.rs around lines 88 to 90, the test file
defines a standalone main() which conflicts with Rust's integration test
harness; remove the main() function (and its println!) so the file only contains
test functions or helper code, ensuring the test harness supplies the entry
point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review continued from previous batch...
| #[derive(ShankAccounts)] | ||
| pub struct CreateTokenAccounts<'info> { | ||
| #[account(mut, signer, desc = "Payer and authority")] | ||
| pub payer: AccountInfo<'info>, | ||
|
|
||
| #[account(mut, desc = "Token mint to create")] | ||
| pub mint: AccountInfo<'info>, | ||
|
|
||
| #[account(desc = "System program")] | ||
| pub system_program: AccountInfo<'info>, | ||
|
|
||
| #[account(desc = "Token program")] | ||
| pub token_program: AccountInfo<'info>, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Fix compile error: visibility qualifiers on inner items (E0449)
Rust forbids pub structs/enums (and the pub methods the macro generates) inside function bodies. Hoist these type/enum definitions to module scope (top of the file) or remove pub and move them out of the test functions. Otherwise, derives will expand to pub impls inside fn and fail to compile.
Apply this pattern (example for the first test; replicate for the rest):
@@
#[test]
fn test_accounts_with_instruction_integration() {
- #[derive(ShankAccounts)]
- pub struct CreateTokenAccounts<'info> {
- #[account(mut, signer, desc = "Payer and authority")]
- pub payer: AccountInfo<'info>,
- #[account(mut, desc = "Token mint to create")]
- pub mint: AccountInfo<'info>,
- #[account(desc = "System program")]
- pub system_program: AccountInfo<'info>,
- #[account(desc = "Token program")]
- pub token_program: AccountInfo<'info>,
- }
-
- #[derive(ShankInstruction)]
- pub enum TokenInstruction {
- #[accounts(CreateTokenAccounts)]
- CreateToken {
- decimals: u8,
- supply: u64,
- },
- }
+ // Types moved to module scope; test body unchanged
let accounts_metadata = CreateTokenAccounts::__shank_accounts();
assert_eq!(accounts_metadata.len(), 4);
@@
}Place near the top of this file (module scope):
#[derive(ShankAccounts)]
pub struct CreateTokenAccounts<'info> {
#[account(mut, signer, desc = "Payer and authority")]
pub payer: AccountInfo<'info>,
#[account(mut, desc = "Token mint to create")]
pub mint: AccountInfo<'info>,
#[account(desc = "System program")]
pub system_program: AccountInfo<'info>,
#[account(desc = "Token program")]
pub token_program: AccountInfo<'info>,
}
#[derive(ShankInstruction)]
pub enum TokenInstruction {
#[accounts(CreateTokenAccounts)]
CreateToken {
decimals: u8,
supply: u64,
},
}Repeat this hoisting for:
- InitializeAccounts, TransferAccounts, CloseAccounts, ProgramInstruction
- BasicAccounts, CreateAccounts, UpdateAccounts, AdminAccounts, ComplexProgram
- ComplexDataAccounts, DataInstruction
- SharedAccounts, SharedInstruction
Also applies to: 29-36, 56-64, 66-80, 82-91, 94-109, 133-136, 139-148, 151-161, 164-174, 178-202, 225-231, 235-251, 262-270, 273-282
🏁 Script executed:
#!/bin/bash
# Verify no inner items with visibility qualifiers remain in this test file
rg -nP '^\s*pub\s+(struct|enum)\s' shank/tests/instruction_integration_test.rs -n -C0
# Expect: only module-scope hits at top-level, none inside #[test] fns.Length of output: 650
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="shank/tests/instruction_integration_test.rs"
echo ">>> Matches (with 6 lines of context):"
rg -n -C6 '^\s*pub\s+(struct|enum)\s' "$FILE" || true
echo
echo ">>> Context blocks around each match (±10 lines):"
rg -n --only-matching '^\s*pub\s+(struct|enum)\s' "$FILE" | cut -d: -f1 | sort -n | uniq | while read -r ln; do
start=$((ln - 10)); if [ "$start" -lt 1 ]; then start=1; fi
end=$((ln + 10))
echo "----- lines $start-$end -----"
sed -n "${start},${end}p" "$FILE" | nl -v${start} -ba -w3 -s": "
doneLength of output: 6422
Hoist pub structs/enums to module scope — fixes E0449
Confirmed: multiple pub struct / pub enum definitions are declared inside #[test] functions in shank/tests/instruction_integration_test.rs. Move these type definitions to module scope (near the top of the file) so derives expand to module-level pub impls; keep test bodies unchanged.
Hoist the following (file:line):
- CreateTokenAccounts — line 14
- TokenInstruction — line 30
- InitializeAccounts — line 56
- TransferAccounts — line 67
- CloseAccounts — line 82
- ProgramInstruction — line 95
- BasicAccounts — line 133
- CreateAccounts — line 140
- UpdateAccounts — line 152
- AdminAccounts — line 165
- ComplexProgram — line 178
- ComplexDataAccounts — line 225
- DataInstruction — line 235
- SharedAccounts — line 262
- SharedInstruction — line 273
🤖 Prompt for AI Agents
In shank/tests/instruction_integration_test.rs around lines 13 to 273, several
pub struct/enum definitions are declared inside #[test] functions which causes
E0449; hoist the listed types to module scope (near the top of the file) so
their derived pub impls expand at module level: move CreateTokenAccounts (line
14), TokenInstruction (line 30), InitializeAccounts (56), TransferAccounts (67),
CloseAccounts (82), ProgramInstruction (95), BasicAccounts (133), CreateAccounts
(140), UpdateAccounts (152), AdminAccounts (165), ComplexProgram (178),
ComplexDataAccounts (225), DataInstruction (235), SharedAccounts (262), and
SharedInstruction (273) out of test functions into top-level module scope while
leaving the test bodies unchanged.
This PR adds a ShankAccounts macro to give the option of sourcing Shank's IDL generation from a struct of AccountInfos rather than the current syntax which can make Instruction enums overly long. The account struct also has an autogenerated context() function which will turn a slice of AccountInfos into the struct.