Conversation
Summary by CodeRabbitRelease Notes
WalkthroughAdds ShankAccounts derive support, expands IDL generation to resolve #[accounts(StructName)] by extracting struct field metadata, generates context() constructors and a generic Context type, wires macro and idl changes across crates, and adds examples plus extensive tests and fixtures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Parser as Crate AST Parser
participant Extract as extract_shank_accounts_structs
participant Map as Accounts Map (HashMap)
participant InstrParser as Instruction Parser
participant IDLConv as IdlInstruction::try_from_with_accounts
Parser->>Extract: iterate ItemStructs
Extract->>Map: insert StructName => Vec<InstructionAccount>
InstrParser->>IDLConv: convert variant with accounts_map
alt Inline accounts on variant
IDLConv->>IDLConv: map inline InstructionAccounts → IdlAccountItems
else Struct reference on variant
IDLConv->>Map: lookup StructName
alt found
Map-->>IDLConv: Vec<InstructionAccount>
IDLConv->>IDLConv: expand to IdlAccountItems
else not found
IDLConv->>IDLConv: emit placeholder IdlAccountItem with docs
end
end
IDLConv-->>InstrParser: IdlInstruction(s)
sequenceDiagram
autonumber
participant Caller as Program code
participant Gen as Generated context() (from ShankAccounts)
participant Ctx as Context<'a, T, U>
Caller->>Gen: call context(&[AccountInfo])
Gen->>Gen: validate min accounts length
Gen->>Gen: for each optional field: if account.key == crate::ID => None else Some(&acc[i])
Gen->>Ctx: construct Context { accounts: T, remaining_accounts }
Ctx-->>Caller: typed ctx.accounts and remaining_accounts
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas needing extra attention:
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
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.
🧹 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.
🛠️ 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.
🧹 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.
🧹 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.
🧹 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.
🧹 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.
shank/tests/simple_context_test.rs
Outdated
| fn main() { | ||
| println!("Simple context test"); | ||
| } No newline at end of file |
There was a problem hiding this comment.
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.
| /// 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.
🧹 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.
| 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.
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.
🛠️ 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.
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.
🧹 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.
| 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.
🧹 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.
| #[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.
🧩 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.
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shank-idl/src/idl_instruction.rs (1)
173-291: Struct‑based accounts resolution viaaccounts_mapis correct but shares duplicated arg parsingThe new
IdlInstruction::try_from_with_accountscorrectly:
- Reuses the same argument naming semantics as the basic
try_from(named vs unnamed, single custom type vsargs,arg{index}for multi‑field tuples).- Handles
AccountsSource::Inlineidentically to the legacy path.- For
AccountsSource::Struct, looks upstruct_nameinaccounts_mapand expands eachInstructionAccountinto anIdlAccountItem, falling back to a clearly documented placeholder when the struct can’t be resolved.Functionally this looks good; the remaining concern is the duplicated arg‑parsing block, which should be factored as noted in the earlier comment.
♻️ Duplicate comments (41)
shank/tests/simple_context_test.rs (1)
32-34: Remove main() from integration test.This issue was already flagged in a previous review. Integration tests should not define a
main()function; test logic should be in#[test]functions instead.shank/tests/instruction_with_accounts_struct_test.rs (1)
28-29: Normalize authority account attribute tooptional, signer(legacyoptional_signer).The
authorityfield is correctly typed asOption<&'info AccountInfo<'info>>, but the attribute still uses the legacyoptional_signerform. For consistency with the rest of the API and attributes in this file, prefer the canonicaloptional, signercombination:- #[account(optional_signer, desc = "Authority on the vault")] - pub authority: Option<&'info AccountInfo<'info>>, + #[account(optional, signer, desc = "Authority on the vault")] + pub authority: Option<&'info AccountInfo<'info>>,Run this to confirm there are no remaining
optional_signerusages:#!/bin/bash rg -n '#\[account\(optional_signer' -C2shank/tests/context_issue_demo.rs (1)
69-71: Removemain()from this integration test; rely on the test harness entry point.Defining
fn main()in a file undertests/conflicts with Rust’s integration test harness, which provides its own main and expects only#[test]functions here. This can cause build failures.You can just delete this function; the tests above will still run via the harness.
-fn main() { - println!("Testing context method availability"); -} +// Integration tests use the harness-provided main; no standalone main needed here.shank/tests/context_method_test.rs (1)
44-48: Avoid tuple positional access in assertions.Indexing the generated metadata with
idl[1].5hard codes the tuple layout; any future change to the derive will silently break this test. Destructure the entry (or look it up by name) and assert on the optional flag explicitly instead.- assert_eq!(idl[1].5, true); // optional + let (_, _, _, _, _, is_optional, _) = idl[1]; + assert!(is_optional, "optional_data account should be optional");shank/tests/working_context_demo.rs (1)
22-35: Replace the integration-testmainwith a#[test].Crates in
tests/already get a generated entry point; leaving a freefn main()here causes a duplicate symbol and the test target fails to compile. Convert this to a named#[test](or move the demo underexamples/) so the suite builds.-fn main() { +#[test] +fn working_context_demo() {shank/tests/context_method_demo.rs (1)
6-9: Drop unusedsolana_programimports.
ProgramErrorandPubkeyaren’t referenced in this module, so the test emits unused-import warnings (and will fail in warning-as-error CI). Trim the import list to keep the test clean.- use solana_program::{ - account_info::AccountInfo, program_error::ProgramError, pubkey::Pubkey, - }; + use solana_program::account_info::AccountInfo;shank/examples/full_solana_example.rs (1)
3-65: DeriveShankTypefor argument structs so they appear in the IDL.The custom payload structs aren’t exported to the IDL unless they derive
ShankType, and our coding guidelines call this out explicitly. Please importShankTypeand derive it on bothCreateVaultArgsandUpdateVaultArgsso the example matches recommended usage.-use shank::{ShankAccounts, ShankInstruction}; +use shank::{ShankAccounts, ShankInstruction, ShankType}; @@ -pub struct CreateVaultArgs { +#[derive(ShankType)] +pub struct CreateVaultArgs { @@ -pub struct UpdateVaultArgs { +#[derive(ShankType)] +pub struct UpdateVaultArgs {shank/tests/expand_tests.rs (1)
1-29: Duplication already noted in previous review.The suggestion to consolidate these test functions to reduce repetition has already been raised in the past review comments.
shank/src/context.rs (1)
1-5: Suggestions already noted in previous review.The recommendations to add a Debug derive, create a convenience alias for AccountInfo, and clarify documentation have already been raised in past review comments.
shank-idl/tests/generate_expected_idl.rs (1)
5-57: Test utilities are functional and well-structured.These ignored tests provide a clean way to regenerate expected IDL JSON files from fixture sources. The pattern is consistent across all three test functions.
Past review comments suggest adding file path printing for debugging (e.g.,
eprintln!("Generating from: {}", file.display());). This remains a valid optional improvement if you want easier correlation between output and source files.shank-idl/tests/test_shank_accounts_expansion.rs (3)
3-5: Test correctly marked as ignored with known limitation.The TODO comment on Line 4 acknowledges that this test requires proper Cargo.toml setup. The current temp file approach will cause
extract_idlto fail when discovering the manifest.
57-61: Temp file approach incompatible with extract_idl.
extract_idldiscovers and reads Cargo.toml to obtain the lib name. Writing a standalone .rs file to the OS temp dir will cause the extraction to fail.The past review comment provides a detailed solution using
tempfileto create a proper Cargo project structure. This remains the correct approach.
78-93: Make assertion strict to catch expansion regressions.The current lenient check allows the test to pass with a warning when expansion fails (accounts.len() == 1). This defeats the purpose of the test.
Apply:
- // Check if we have the expected accounts expanded - if instruction.accounts.len() == 1 { - // If we still have only 1 account, it means our expansion didn't work yet - println!("WARNING: Accounts not expanded yet - still showing struct placeholder"); - } else { - // Success! We have multiple accounts - println!( - "SUCCESS: Accounts expanded to {} individual accounts", - instruction.accounts.len() - ); - // We expect 7 accounts from the CreateMachineV1Accounts struct - assert_eq!( - instruction.accounts.len(), - 7, - "Should have 7 individual accounts" - ); - } + // We expect 7 accounts expanded from CreateMachineV1Accounts + println!("Accounts: {:#?}", instruction.accounts); + assert_eq!( + instruction.accounts.len(), + 7, + "Accounts must be expanded from struct to 7 individual accounts" + );shank-idl/tests/instructions.rs (1)
194-243: New tests follow established patterns correctly.The three new test functions validate IDL generation for simple, standard, and complex accounts struct scenarios. Each test correctly:
- Constructs the fixture path
- Parses with
optional_program_address()config- Compares against expected JSON using
include_str!The past review comment suggesting a helper function to reduce boilerplate remains a valid optional refactor for maintainability.
shank/tests/idl_generation_test.rs (1)
12-292: Comprehensive test coverage for IDL generation.The test suite thoroughly validates ShankAccounts IDL generation across diverse scenarios:
- Basic and complex account constraints
- Optional accounts and signers
- Custom lifetimes
- Edge cases (empty struct, single account)
- Indexing and field order preservation
All assertions are correct and the test coverage is excellent.
The past review comment suggesting a type alias (e.g.,
type AccountMeta = (u32, &'static str, bool, bool, bool, bool, Option<String>)) to reduce tuple-indexing noise remains a valid optional improvement for readability.shank/tests/basic_context_test.rs (2)
38-39: Fix: Missing lifetime parameter in Context type annotation.The type annotation is missing the required lifetime parameter for
BasicTestAccounts.Apply:
- let ctx: Context<BasicTestAccounts, AccountInfo> = - BasicTestAccounts::context(&accounts); + let ctx: Context<BasicTestAccounts<'_>, AccountInfo<'_>> = + BasicTestAccounts::context(&accounts);Or simply omit the type annotation:
- let ctx: Context<BasicTestAccounts, AccountInfo> = - BasicTestAccounts::context(&accounts); + let ctx = BasicTestAccounts::context(&accounts);
46-48: Remove main() from integration test.Integration tests run under Cargo's test harness; defining
fn main()causes a conflict.Apply:
-fn main() { - println!("Basic context test"); -}shank/tests/expand/no_constraints.expanded.rs (1)
1-1: Remove unused import to avoid compiler warnings.The
use shank::ShankAccounts;import on Line 1 is not referenced in this expanded file and will triggerunused_importswarnings in stricter CI environments.Apply:
-use shank::ShankAccounts;Alternatively, if the import must remain for fixture consistency, add:
+#[allow(unused_imports)] use shank::ShankAccounts;shank-macro-impl/src/accounts/account_struct.rs (1)
7-9: DeriveDebugonAccountStructfor easier diagnosticsDeriving
DebugonAccountStructmakes it much easier to inspect field state during tests or when debugging macro expansions.- pub struct AccountStruct { + #[derive(Debug)] + pub struct AccountStruct { pub fields: Vec<AccountField>, }[ suggest_optional_refactor]
shank-idl/src/idl_instruction.rs (1)
69-170: Fix discriminant bound off‑by‑one and deduplicate arg parsingTwo issues in
TryFrom<InstructionVariant>:
- Discriminant off‑by‑one (also present in
try_from_with_accountsbelow):- 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 ) );Apply the same change in the
ensure!at lines 272–281 so both constructors accept 255 as valid while still guarding the cast tou8.
- Duplicate arg‑parsing logic:
The
InstructionVariantFields→Vec<IdlField>logic here is essentially identical to the block intry_from_with_accounts. Extracting a small helper, e.g.:fn parse_args(field_tys: InstructionVariantFields) -> Result<Vec<IdlField>, Error> { ... }and calling it from both places will keep behavior in sync and reduce maintenance risk.
shank-macro-impl/src/accounts/account_field.rs (4)
15-21: Avoidunwrap()onfield.ident; emit a proper compile error for unnamed fields
field.ident.clone().unwrap()will panic for tuple/unnamed fields. For a derive macro, this should be asyn::Errorinstead of a panic.- pub fn from_field(field: &Field) -> Result<Self> { - let name = field.ident.clone().unwrap(); + pub fn from_field(field: &Field) -> Result<Self> { + let name = field.ident.clone().ok_or_else(|| { + syn::Error::new_spanned(field, "ShankAccounts requires named struct fields") + })?;
23-51: Validate attribute names more clearly and support future aliases safelyThe core parsing looks fine, but two improvements will make diagnostics and future extension safer:
- Unknown attributes: printing
path.get_ident()yieldsOption<Ident>in the message.-use quote::quote; +use quote::{quote, ToTokens}; @@ - } else { - return Err(syn::Error::new_spanned( - &path, - format!( - "Unknown account attribute: {:?}", - path.get_ident() - ), - )); - } + } else { + return Err(syn::Error::new_spanned( + &path, + format!( + "Unknown account attribute: {}", + path.to_token_stream() + ), + )); + }
- Consider explicitly documenting here (or in the ShankAccounts docs) which aliases are supported (
mut,writ,w, etc.) so users know what’s intentional vs. typo.
61-72: Reject non‑stringdesc/description/docsliterals instead of silently ignoringCurrently, non‑string literals for
descare ignored, which hides mistakes:#[account(desc = 123)] // silently droppedTreat this as a hard error so users get immediate feedback:
- )) => { - if path.is_ident("desc") - || path.is_ident("description") - || path.is_ident("docs") - { - if let Lit::Str(s) = lit { - desc = Some(s.value()); - } - } - } + )) => { + if path.is_ident("desc") + || path.is_ident("description") + || path.is_ident("docs") + { + match lit { + Lit::Str(s) => desc = Some(s.value()), + other => { + return Err(syn::Error::new_spanned( + other, + "desc/docs/description must be a string literal", + )); + } + } + } + }
15-21: Enforce#[account(optional)]/#[account(optional_signer)]only onOption<...>fieldsRight now
optionalandoptional_signerare just booleans; they don’t validate the field type. That allows:#[account(optional)] pub bad: &'info AccountInfo<'info>;which will be surprising at runtime when the account is actually missing. Add a type check so these attributes only apply to
Option<...>fields:-use syn::{Field, Ident, Lit, Meta, MetaNameValue, NestedMeta, Result}; +use syn::{Field, Ident, Lit, Meta, MetaNameValue, NestedMeta, Result, Type, PathArguments}; @@ pub fn from_field(field: &Field) -> Result<Self> { - let name = field.ident.clone().unwrap(); + let name = field.ident.clone().ok_or_else(|| { + syn::Error::new_spanned(field, "ShankAccounts requires named struct fields") + })?; @@ - // Validate mutually exclusive attributes - if signer && optional_signer { + // Validate mutually exclusive attributes + if signer && optional_signer { return Err(syn::Error::new_spanned( &name, "Account cannot be both 'signer' and 'optional_signer'", )); } + + // Validate that optional flags match Option<T> field types + if optional || optional_signer { + let is_option = matches!( + &field.ty, + Type::Path(tp) + if tp.path.segments.last().map(|s| s.ident == "Option").unwrap_or(false) + && matches!( + tp.path.segments.last().unwrap().arguments, + PathArguments::AngleBracketed(_) + ) + ); + + if !is_option { + return Err(syn::Error::new_spanned( + &field.ty, + "#[account(optional)] and #[account(optional_signer)] require the field type to be Option<...>", + )); + } + }This matches the attribute’s intent and prevents subtle runtime issues.
shank/tests/context_runtime_test.rs (1)
93-126: Add assertions foroptional_signerflags in ComplexAccounts metadata
ComplexAccounts<'info>includes an#[account(optional_signer, ...)]field, but the test only checks lengths and first-field names. Prior review recommended asserting the optional-signer semantics to guard against regressions in__shank_accounts()generation.
[ suggest_recommended_refactor ]You could extend the test like this:
let simple_metadata = SimpleAccounts::__shank_accounts(); let complex_metadata = ComplexAccounts::__shank_accounts(); @@ assert_eq!(simple_metadata.len(), 1); assert_eq!(complex_metadata.len(), 4); @@ assert_eq!(simple_metadata[0].1, "user"); assert_eq!(complex_metadata[0].1, "authority"); + + // Verify optional_signer semantics + assert_eq!(complex_metadata[2].1, "optional_signer"); + assert_eq!(complex_metadata[2].3, true); // signer + assert_eq!(complex_metadata[2].4, true); // optional_signer flag + assert_eq!(complex_metadata[2].5, true); // optional impliedAdjust indices if the field order changes.
shank-macro-impl/src/accounts/mod.rs (1)
51-71: Consider matching __shank_accounts Vec construction with existing expand tests
__shank_accounts()currently builds its Vec withvec![ #account_list ], while existing.expanded.rsfixtures use the boxed-arrayinto_vecpattern (Box::new([ .. ]).into_vec()).
[ suggest_optional_refactor ]If your expand tests assert textual equality, you may want to align:
- pub fn __shank_accounts() -> Vec<(u32, &'static str, bool, bool, bool, bool, Option<String>)> { - vec![ - #account_list - ] - } + pub fn __shank_accounts() -> Vec<(u32, &'static str, bool, bool, bool, bool, Option<String>)> { + <[_]>::into_vec( + #[rustc_box] + ::alloc::boxed::Box::new([ + #account_list + ]), + ) + }If tests compare behavior only, the current
vec!form is fine.shank-idl/src/accounts_extraction.rs (3)
30-61: Doc comment still describes calling__shank_accountsrather than parsing attributesThis helper now parses struct fields and their
#[account(...)]attributes; it does not call any generated__shank_accounts()method. The doc comment is misleading and previously flagged.Consider updating it along these lines:
-/// 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
77-86: Option detection should use the last path segment, not the first
has_option_typecheckspath.segments.first() == "Option", which fails for fully-qualified types likestd::option::Option<T>. Using the last segment is more robust and was previously suggested.You can tighten this with:
- 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"));
88-116: String-based#[account(...)]parsing is brittle; use syn’s nested-meta APIAttribute handling via
tokens_str.contains(...)is whitespace/order-sensitive and can misfire on substrings in descriptions. Prior review already called this out and suggestedAttribute::parse_nested_meta.A more robust approach (adapt paths for your syn version) would be:
- // 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 + for attr in &field.attrs { + // Adjust to attr.path / attr.path() depending on syn version in use. + 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(()) + })?; + } + }Please double-check against your
synversion and feature flags when adopting this.shank/tests/final_context_test.rs (1)
96-98: Removemain()from integration test fileIntegration tests are built with the test harness’ own
main; defining your ownfn main()inshank/tests/final_context_test.rscan conflict with the harness and is unnecessary.You can simply drop it:
-fn main() { - println!("Final context test - context method is working!"); -} +// No custom main() in integration tests; use #[test] functions instead.shank/tests/expand/complex_constraints.rs (1)
34-34: Removemain()from expand test fixtureAs with other test files, a top-level
fn main()intests/expand/complex_constraints.rsis unnecessary and can clash with the test harness.Suggested change:
-fn main() {} +// No main() in expand tests; expansion is verified via macrotest.shank/tests/context_api_working_test.rs (1)
117-119: Removemain()from integration test crate
tests/context_api_working_test.rsis compiled as an integration test crate with a test harness–generatedmain(). Defining anotherfn main()will conflict and prevent tests from compiling; drop this function so the file only contains#[test]functions.-fn main() { - println!("Context API working test"); -} +// No `main()` in integration tests; test harness provides the entry point.shank/tests/accounts_macro_test.rs (1)
13-80: Consider minimally asserting on generated APIs to catch regressionsThese tests currently only ensure the
ShankAccountsderive compiles. To better guard against regressions (e.g.,__shank_accounts/contextno longer being emitted), consider adding a trivial use of a generated symbol, for example:let _ = CreateVaultAccounts::__shank_accounts();or
let _ctx_fn: fn(&[AccountInfo]) -> shank::Context<_, _> = CreateVaultAccounts::context;Same applies to
ComplexAccounts/AlternativeAccounts.shank/tests/context_api_test.rs (1)
15-59: Align authority attribute withoptional_signersemantics and assert description fieldIn
CreateMachineV1Accounts<'info>:
- The comment and later assertion (
metadata[4]treated as an optional signer) implyoptional_signersemantics, but the field is annotated as#[account(optional, signer, ...)]. Prefer using the combined form to exercise that code path explicitly:- #[account( - optional, - signer, - desc = "The authority signing for account creation" - )] + #[account( + optional_signer, + desc = "The authority signing for account creation" + )] pub authority: Option<&'info AccountInfo<'info>>,
- To better lock the tuple layout, add a description assertion for the first entry:
assert_eq!(metadata[0].1, "machine"); assert_eq!(metadata[0].2, true); // mut assert_eq!(metadata[0].3, true); // signer + assert_eq!( + metadata[0].6.as_deref(), + Some("The new machine asset account") + );shank/tests/expand/custom_lifetime.expanded.rs (1)
1-1: Remove unusedShankAccountsimport from expanded test.This expanded file no longer uses the derive macro; keeping
use shank::ShankAccounts;just introduces an unused-import warning. It’s safe to drop.-use shank::ShankAccounts;shank/tests/expand/optional_accounts.expanded.rs (1)
1-1: Drop unusedShankAccountsimport from expanded optional-accounts test.The derive is already expanded; this
use shank::ShankAccounts;is unused and can be removed to avoid warnings.-use shank::ShankAccounts;shank/tests/context_usage_test.rs (1)
41-73: Fix&mut <vec>.clone()when constructingAccountInfoto avoid borrowing temporaries.Across these tests you build data buffers as
vec![]and then pass&mut <buffer>.clone()intoAccountInfo::new. That clones into a temporary and borrows it mutably for the account’s data, instead of borrowing a stable buffer variable.This is both unnecessary and potentially unsound; use named mutable buffers and pass them directly.
Apply the pattern below to all four test cases (required accounts, optional account, program_id placeholder, not-enough-accounts):
- let payer_data = vec![]; - let mint_data = vec![]; - let system_data = vec![]; + let mut payer_data: Vec<u8> = vec![]; + let mut mint_data: Vec<u8> = vec![]; + let mut system_data: Vec<u8> = vec![]; @@ - &mut payer_lamports, - &mut payer_data.clone(), + &mut payer_lamports, + &mut payer_data, @@ - &mut mint_lamports, - &mut mint_data.clone(), + &mut mint_lamports, + &mut mint_data, @@ - &mut system_lamports, - &mut system_data.clone(), + &mut system_lamports, + &mut system_data,Extend the same change to any additional
*_databuffers in the other tests (metadata_data,program_data, etc.): declare them aslet mut ...: Vec<u8> = vec![];and pass&mut ...instead of&mut ... .clone().Also applies to: 103-151, 177-225, 248-271
shank-macro-impl/src/instruction/account_attrs.rs (1)
199-225: Reject multiple#[accounts(...)]attributes instead of silently taking the first.
AccountsSource::try_fromcurrently uses.find(...), so if a variant accidentally has more than one#[accounts(...)]attribute, you’ll just pick the first and ignore the rest. That’s ambiguous and hard to debug.Consider collecting all such attrs, erroring on duplicates, and then using the single allowed one, e.g.:
- // First check for #[accounts(StructName)] attribute - let accounts_struct_attr = attrs - .iter() - .find(|attr| AccountsSource::is_accounts_struct_attr(attr)); - - if let Some(attr) = accounts_struct_attr { + // First check for #[accounts(StructName)] attribute(s) + let struct_attrs: Vec<&Attribute> = attrs + .iter() + .filter(|attr| AccountsSource::is_accounts_struct_attr(attr)) + .collect(); + + if let Some(attr) = struct_attrs.first() { + if struct_attrs.len() > 1 { + return Err(ParseError::new_spanned( + struct_attrs[1], + "Multiple #[accounts(...)] attributes found; only one is allowed", + )); + } // Can't have both #[accounts(StructName)] and #[account(...)] attributes - let has_inline_accounts = attrs.iter().any(|attr| { - InstructionAccount::is_account_attr(attr).is_some() - }); + let has_inline_accounts = attrs + .iter() + .any(|attr| InstructionAccount::is_account_attr(attr).is_some());The rest of the logic (struct vs. inline exclusivity, and falling back to inline accounts) can stay as-is.
shank/examples/accounts_struct_example.rs (2)
48-51: Clarify that optional accounts becomeOption<...>in the generated context.Here
new_authorityis aPhantomData<()>placeholder, but at runtime the generated context will expose it as anOption<&AccountInfo<'info>>. Without a note, readers might copy the PhantomData pattern into real programs.- /// Optional new authority - #[account(optional, desc = "Optional new authority")] - pub new_authority: std::marker::PhantomData<()>, + /// Optional new authority + #[account(optional, desc = "Optional new authority")] + pub new_authority: std::marker::PhantomData<()>, // In generated context: Option<&AccountInfo<'info>>
120-124: Soften “complete replacement” wording forShankAccountsvsShankContext.The example currently claims ShankAccounts is a “complete replacement” for the older patterns, but docs/guides still reference
ShankContext. To avoid over-claiming and confusing users, this should be phrased as an alternative/more ergonomic option rather than a full replacement.- println!("ShankAccounts is designed to be a complete replacement for:"); + println!("ShankAccounts provides a more ergonomic alternative to:"); @@ - println!("- ShankContext derive macro for context generation"); + println!("- ShankContext derive macro for context generation (still supported)");shank/tests/instruction_integration_test.rs (1)
12-36: Hoist allpubaccount/Instruction types out of tests (same issue as prior review).Each
#[test]definespub struct/pub enumitems inside the function body:
CreateTokenAccounts<'info>/TokenInstructionInitializeAccounts<'info>/TransferAccounts<'info>/CloseAccounts<'info>/ProgramInstructionBasicAccounts<'info>/CreateAccounts<'info>/UpdateAccounts<'info>/AdminAccounts<'info>/ComplexProgramComplexDataAccounts<'info>/DataInstructionSharedAccounts<'info>/SharedInstructionThis pattern was already flagged earlier: Rust does not allow
pubitems at function scope, and the derives generate additionalpubAPIs, so these tests will either not compile or rely on undefined behavior of the macro expansions.Apply the same fix suggested previously:
- Move all these derived types to module scope (top of the file), keeping them
pub.- Leave the test bodies to call
__shank_accounts()/ use the enums, without redefining the types inside the functions.Example pattern for the first test:
-use shank::{ShankAccounts, ShankInstruction}; - -// Mock AccountInfo for testing -pub struct AccountInfo<'info> { /* ... */ } - -pub const ID: [u8; 32] = [1; 32]; - -#[test] -fn test_accounts_with_instruction_integration() { - // Define accounts struct - #[derive(ShankAccounts)] - pub struct CreateTokenAccounts<'info> { /* ... */ } - - // Use accounts struct in instruction - #[derive(ShankInstruction)] - pub enum TokenInstruction { /* ... */ } - - let accounts_metadata = CreateTokenAccounts::__shank_accounts(); - // ... -} +use shank::{ShankAccounts, ShankInstruction}; + +// Mock AccountInfo for testing +pub struct AccountInfo<'info> { /* ... */ } + +pub const ID: [u8; 32] = [1; 32]; + +#[derive(ShankAccounts)] +pub struct CreateTokenAccounts<'info> { /* ... */ } + +#[derive(ShankInstruction)] +pub enum TokenInstruction { /* ... */ } + +#[test] +fn test_accounts_with_instruction_integration() { + let accounts_metadata = CreateTokenAccounts::__shank_accounts(); + // ... +}Repeat this hoisting for the other account structs and instruction enums defined in the remaining tests.
After hoisting, re-check with:
#!/bin/bash rg -nP '^\s*pub\s+(struct|enum)\s' shank/tests/instruction_integration_test.rs -n -C2 # Expect all matches at module scope, outside #[test] fns.Also applies to: 53-103, 127-193, 215-243, 250-273
| fn gen_context_impl(&self) -> TokenStream { | ||
| let ident = &self.ident; | ||
| let (impl_gen, type_gen, where_clause) = self.generics.split_for_impl(); | ||
| let fields = &self.account_struct.fields; | ||
|
|
||
| // All accounts must be provided, but optional ones can be program_id placeholders | ||
| let expected_accounts = fields.len(); | ||
| let _total_accounts = fields.len(); | ||
|
|
||
| // Use the same lifetime as the struct, or skip context method if no lifetimes | ||
| let context_method = if let Some(lifetime) = | ||
| self.generics.lifetimes().next() | ||
| { | ||
| let lifetime_ident = &lifetime.lifetime; | ||
|
|
||
| let account_assignments = | ||
| fields.iter().enumerate().map(|(idx, field)| { | ||
| let field_name = &field.name; | ||
| if field.optional || field.optional_signer { | ||
| quote! { | ||
| #field_name: if accounts[#idx].key == &crate::ID { | ||
| None | ||
| } else { | ||
| Some(&accounts[#idx]) | ||
| } | ||
| } | ||
| } else { | ||
| quote! { | ||
| #field_name: &accounts[#idx] | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| quote! { | ||
| /// Create a context from a slice of accounts | ||
| /// | ||
| /// This method parses the accounts according to the struct definition | ||
| /// and returns a Context containing the account struct. | ||
| /// | ||
| /// Optional accounts are determined by checking if the account key | ||
| /// equals the program ID (crate::ID). If so, they are set to None, otherwise Some. | ||
| pub fn context( | ||
| accounts: &#lifetime_ident [AccountInfo<#lifetime_ident>] | ||
| ) -> ::shank::Context<#lifetime_ident, Self, AccountInfo<#lifetime_ident>> { | ||
| if accounts.len() < #expected_accounts { | ||
| panic!("Expected at least {} accounts, got {}", #expected_accounts, accounts.len()); | ||
| } | ||
|
|
||
| let account_struct = Self { | ||
| #(#account_assignments,)* | ||
| }; | ||
|
|
||
| ::shank::Context { | ||
| accounts: account_struct, | ||
| remaining_accounts: &accounts[#expected_accounts..], | ||
| } | ||
| } | ||
| } | ||
| } else { | ||
| // No lifetime parameters, don't generate context method | ||
| quote! {} | ||
| }; | ||
|
|
||
| quote! { | ||
| impl #impl_gen #ident #type_gen #where_clause { | ||
| #context_method | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
context() API: panic behavior, crate::ID coupling, and optional account handling
The generated context() currently:
- Panics on short
accountsslices instead of returning aResult. - Keys optional-ness off
accounts[idx].key == &crate::ID, hard-wiring a crate-level ID sentinel. - Always requires at least one account per field (
expected_accounts = fields.len()), even for optional fields. - Slices
remaining_accountsfromexpected_accounts, ignoring how many entries were actually consumed.
This matches the current expansion fixtures, but it’s at odds with common Solana patterns (using program_id and ProgramError::NotEnoughAccountKeys) and makes the API brittle for callers.
Consider evolving the API (likely in a follow-up change) along these lines:
- Gate on the Solana feature when you start depending on
solana_programtypes:
#[cfg(feature = "solana-program")]
pub fn context(
accounts: &'info [AccountInfo<'info>],
program_id: &solana_program::pubkey::Pubkey,
) -> core::result::Result<
::shank::Context<'info, Self, AccountInfo<'info>>,
solana_program::program_error::ProgramError,
> { /* ... */ }- Compute
min_requiredas the count of non-optional, non-optional_signer fields, and returnProgramError::NotEnoughAccountKeyswhenaccounts.len() < min_requiredinstead of panicking. - For optional fields, bounds-check
idx < accounts.len()and treat out-of-range indices asNone. - Compare against
program_id(or another explicit sentinel parameter) rather thancrate::ID, so the macro doesn’t assume a global constant exists in the crate using it. - Use
used = core::cmp::min(accounts.len(), total_accounts)and sliceremaining_accountsfromusedto avoid accidental OOB if future changes relax the length checks.
This would make the generated API safer and closer to Solana runtime conventions; please coordinate with existing call sites and tests before changing the signature.
I'll verify the review comment by examining the implementation, tests, and call sites. Let me gather the necessary information.
🏁 Script executed:
#!/bin/bash
# 1. Check the actual implementation at the specified lines
echo "=== Checking current implementation ==="
sed -n '73,141p' shank-macro-impl/src/accounts/mod.rs
echo -e "\n=== Looking for context() tests and fixtures ==="
fd -e rs -x grep -l "context()" {} \; | head -20
echo -e "\n=== Checking Cargo.toml for dependencies and features ==="
cat Cargo.toml | head -50Length of output: 3401
🏁 Script executed:
#!/bin/bash
# 2. Search for context() method usages in the codebase
echo "=== Finding context() usages ==="
rg -nC 3 'context\s*\(' shank-macro-impl/src --type rs | head -40
echo -e "\n=== Looking for ProgramError usage patterns ==="
rg -nC 2 'ProgramError|NotEnoughAccountKeys' --type rs | head -30
echo -e "\n=== Checking for Result<> patterns in similar macros ==="
rg -nC 2 'fn.*->.*Result' shank-macro-impl/src --type rs | head -30Length of output: 220
🏁 Script executed:
#!/bin/bash
# 3. Look for tests specific to context generation
echo "=== Finding tests for context method ==="
fd -e rs -path '*/test*' -o -path '*/tests/*' | xargs grep -l "context" 2>/dev/null | head -10
echo -e "\n=== Checking test fixtures ==="
fd -e rs -path '*/fixtures/*' -o -path '*/snapshots/*' | head -20
# 4. Check if shank crate has solana_program as a feature
echo -e "\n=== Checking root Cargo.toml for features ==="
fd Cargo.toml | xargs grep -A 10 '\[features\]' | head -30Length of output: 572
🏁 Script executed:
#!/bin/bash
# 5. Look at the complete gen_context_impl function and surrounding context
echo "=== Full context around gen_context_impl ==="
sed -n '50,150p' shank-macro-impl/src/accounts/mod.rsLength of output: 3578
🏁 Script executed:
#!/bin/bash
# Look at test files to understand current usage
echo "=== Content of basic_context_test.rs ==="
cat shank/tests/basic_context_test.rs 2>/dev/null | head -100
echo -e "\n=== Content of context_issue_demo.rs ==="
cat shank/tests/context_issue_demo.rs 2>/dev/null | head -100
echo -e "\n=== Content of working_context_demo.rs ==="
cat shank/tests/working_context_demo.rs 2>/dev/null | head -100Length of output: 4223
🏁 Script executed:
#!/bin/bash
# Search for context() calls across all .rs files
echo "=== All context() method calls ==="
find . -name "*.rs" -type f -exec grep -n "\.context(" {} + 2>/dev/null | head -40
echo -e "\n=== Checking if crate::ID is referenced in tests ==="
find . -name "*.rs" -type f -exec grep -n "crate::ID" {} + 2>/dev/null | head -20Length of output: 2572
🏁 Script executed:
#!/bin/bash
# Check the structure and what shank crate exports
echo "=== shank/src/lib.rs or main entry ==="
head -80 shank/src/lib.rs 2>/dev/null || head -80 shank/src/main.rs 2>/dev/null
echo -e "\n=== Checking for ID definition in shank crate ==="
find shank/src -name "*.rs" -type f -exec grep -l "^pub.*ID\|pub const ID" {} + 2>/dev/null
echo -e "\n=== Looking for panic! in current generated code in tests ==="
find . -path "*/target" -prune -o -name "*.rs" -type f -exec grep -n "panic.*Expected at least" {} + 2>/dev/nullLength of output: 554
🏁 Script executed:
#!/bin/bash
# Check if there are integration tests that would break with Result signature
echo "=== Enhanced accounts test ==="
cat shank/tests/enhanced_accounts_test.rs 2>/dev/null | head -150
echo -e "\n=== Looking for programs that use context() ==="
cat shank/examples/context_usage_example.rs 2>/dev/null | head -100Length of output: 7096
🏁 Script executed:
#!/bin/bash
# Check dependencies in shank-macro-impl/Cargo.toml
echo "=== shank-macro-impl dependencies ==="
cat shank-macro-impl/Cargo.toml
echo -e "\n=== shank-macro Cargo.toml ==="
cat shank-macro/Cargo.toml 2>/dev/null | head -40Length of output: 1155
🏁 Script executed:
#!/bin/bash
# Check macro expansion snapshots to see what is ACTUALLY generated
echo "=== Checking macro expansion snapshot ==="
ls -la shank/tests/expand/ 2>/dev/null | head -20
echo -e "\n=== Complex constraints expanded output (shows generated context) ==="
head -150 shank/tests/expand/complex_constraints.expanded.rs 2>/dev/null
echo -e "\n=== Optional accounts expanded ==="
sed -n '60,90p' shank/tests/expand/optional_accounts.expanded.rs 2>/dev/nullLength of output: 6617
🏁 Script executed:
#!/bin/bash
# Check render_context.rs since it was found in search
echo "=== shank-render/src/context/render_context.rs ==="
cat shank-render/src/context/render_context.rs 2>/dev/null | head -150
echo -e "\n=== Checking what's imported/re-exported in shank-macro ==="
grep -n "render_context\|Context" shank-macro/src/lib.rs 2>/dev/nullLength of output: 2972
🏁 Script executed:
#!/bin/bash
# Check test configuration - are test files actually run?
echo "=== Cargo.toml [[test]] sections ==="
grep -A 5 "\[\[test\]\]" shank/Cargo.toml 2>/dev/null
echo -e "\n=== Check if example files are compiled ==="
grep -A 5 "\[\[example\]\]" shank/Cargo.toml 2>/dev/null
echo -e "\n=== Are context tests in lib or separate ==="
head -20 shank/src/lib.rs 2>/dev/null
# Check if generated code snapshot is what macro currently outputs
echo -e "\n=== First 50 lines of optional_accounts.expanded (shows actual output) ==="
head -50 shank/tests/expand/optional_accounts.expanded.rs 2>/dev/nullLength of output: 2078
🏁 Script executed:
#!/bin/bash
# Verify the actual function used - is it from accounts/mod.rs or render_context.rs?
echo "=== Searching for where gen_context_impl is called ==="
grep -rn "gen_context_impl\|render_context" shank-macro/src shank-macro-impl/src 2>/dev/null
echo -e "\n=== Check shank-render for context generation ==="
head -100 shank-render/src/lib.rs 2>/dev/nullLength of output: 452
Verify implementation concerns in context() method—all four identified issues are confirmed
The review comment accurately identifies real issues in the current context() implementation for ShankAccounts:
-
Panic instead of Result: The macro expansion snapshots confirm that generated code calls
panic!("Expected at least {} accounts, got {}", ...)whenaccounts.len() < expected_accounts. This is visible inshank/tests/expand/complex_constraints.expanded.rs(lines ~125-135). -
crate::IDcoupling: The macro hardcodesaccounts[idx].key == &crate::IDfor optional field detection, confirmed in all.expanded.rssnapshots. Theshankcrate does not defineIDitself; users must define it in their program, making this coupling implicit and fragile. -
All fields required even if optional:
expected_accounts = fields.len()includes optional fields, so the generated code panics if fewer than all field slots are provided. Optional fields should only require bounds-checking, not panic-on-short-slice. -
Unsafe
remaining_accountsslicing: The code slices fromexpected_accountswithout validating the actual count of consumed entries, risking out-of-bounds access if optional field handling changes.
Additionally, an alternate implementation exists in shank-render/src/context/render_context.rs that already returns Result<Context, ProgramError::NotEnoughAccountKeys>, but it still relies on crate::ID and has the same optional field issues.
The test files (context_usage_example.rs, enhanced_accounts_test.rs) show the desired API signature but don't match the current implementation, indicating the concerns are well-founded aspirations for API improvement.
🤖 Prompt for AI Agents
In shank-macro-impl/src/accounts/mod.rs around lines 73–141, the generated
context() currently panics on short slices, hardcodes crate::ID for
optional-account detection, treats optional fields as required
(expected_accounts = fields.len()), and unsafely slices remaining_accounts;
change the generated method to return Result<::shank::Context<...>,
ProgramError> rather than panicking, accept a program_id parameter (or take it
from a passed-in Context/trait) and use program_id for optional checks instead
of crate::ID, compute expected_accounts as the number of non-optional fields
(only assert/return Err when accounts.len() < required_count), build each field
using safe indexing (e.g., check length before accessing each index or use get
and map) so optional fields can be None when their key equals program_id, and
compute consumed = number of assigned slots then safely slice remaining_accounts
using checked bounds before constructing and returning Ok(context).
| let (accounts_source, accounts, strategies) = | ||
| match IdlInstruction::try_from(attrs) { | ||
| Ok(idl_ix) => { | ||
| field_tys = idl_ix.to_instruction_fields(ident.clone()); | ||
| let accounts = idl_ix.to_accounts(ident.clone()); | ||
| ( | ||
| AccountsSource::Inline(accounts.clone()), | ||
| accounts.0, | ||
| InstructionStrategies( | ||
| HashSet::<InstructionStrategy>::new(), | ||
| ), | ||
| ) | ||
| } | ||
| 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, | ||
| }] | ||
| } | ||
| }; | ||
| let strategies: InstructionStrategies = attrs.into(); | ||
| (accounts_source, accounts, strategies) | ||
| } | ||
| }; |
There was a problem hiding this comment.
Don't drop instruction strategy attributes
When IdlInstruction::try_from(attrs) succeeds we now hard-code strategies to an empty set, so any additional instruction-level attributes (e.g. #[legacy_optional_accounts]) silently stop working for those variants. Please keep parsing strategies from attrs in this branch just like the fallback path, so downstream logic still sees them.
(
AccountsSource::Inline(accounts.clone()),
accounts.0,
- InstructionStrategies(
- HashSet::<InstructionStrategy>::new(),
- ),
+ InstructionStrategies::from(attrs),
)📝 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 (accounts_source, accounts, strategies) = | |
| match IdlInstruction::try_from(attrs) { | |
| Ok(idl_ix) => { | |
| field_tys = idl_ix.to_instruction_fields(ident.clone()); | |
| let accounts = idl_ix.to_accounts(ident.clone()); | |
| ( | |
| AccountsSource::Inline(accounts.clone()), | |
| accounts.0, | |
| InstructionStrategies( | |
| HashSet::<InstructionStrategy>::new(), | |
| ), | |
| ) | |
| } | |
| 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, | |
| }] | |
| } | |
| }; | |
| let strategies: InstructionStrategies = attrs.into(); | |
| (accounts_source, accounts, strategies) | |
| } | |
| }; | |
| let (accounts_source, accounts, strategies) = | |
| match IdlInstruction::try_from(attrs) { | |
| Ok(idl_ix) => { | |
| field_tys = idl_ix.to_instruction_fields(ident.clone()); | |
| let accounts = idl_ix.to_accounts(ident.clone()); | |
| ( | |
| AccountsSource::Inline(accounts.clone()), | |
| accounts.0, | |
| InstructionStrategies::from(attrs), | |
| ) | |
| } | |
| 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, | |
| }] | |
| } | |
| }; | |
| let strategies: InstructionStrategies = attrs.into(); | |
| (accounts_source, accounts, strategies) | |
| } | |
| }; |
🤖 Prompt for AI Agents
In shank-macro-impl/src/instruction/instruction.rs around lines 134 to 176, the
match arm for Ok(idl_ix) currently hard-codes strategies to an empty set which
drops any instruction-level attributes; instead parse strategies from attrs the
same way as the Err branch (e.g. let strategies: InstructionStrategies =
attrs.into();) and return that strategies value in the tuple so downstream logic
still receives any instruction strategy attributes.
| // ----------------- | ||
| // #[derive(ShankAccounts)] | ||
| // ----------------- | ||
|
|
||
| /// Annotates a _struct_ that defines accounts for an instruction in a similar way to Anchor. | ||
| /// | ||
| /// This is designed as a complete replacement for both: | ||
| /// - The `#[account]` attribute system on instruction enums | ||
| /// - The `ShankContext` derive macro for context generation | ||
| /// | ||
| /// Instead of annotating instruction variants directly, you define a separate struct | ||
| /// that contains all accounts with their constraints. This generates both IDL metadata | ||
| /// and runtime context handling code for type-safe account access. | ||
| /// | ||
| /// # Field Attributes | ||
| /// | ||
| /// Each field in the struct represents an account and can be annotated with attributes: | ||
| /// | ||
| /// - `#[account(writable)]` or `#[account(mut)]` - The account is writable | ||
| /// - `#[account(signer)]` - The account must sign the transaction | ||
| /// - `#[account(optional_signer)]` - The account may optionally sign | ||
| /// - `#[account(optional)]` - The account is optional (defaults to program_id when not provided) | ||
| /// - `#[account(desc = "...")]` - Description of the account for documentation | ||
| /// | ||
| /// # Example | ||
| /// | ||
| /// ```ignore | ||
| /// use shank::ShankAccounts; | ||
| /// | ||
| /// #[derive(ShankAccounts)] | ||
| /// pub struct CreateVaultAccounts { | ||
| /// #[account(mut, desc = "Initialized fractional share mint")] | ||
| /// pub fraction_mint: std::marker::PhantomData<()>, | ||
| /// | ||
| /// #[account(mut, desc = "Initialized redeem treasury")] | ||
| /// pub redeem_treasury: std::marker::PhantomData<()>, | ||
| /// | ||
| /// #[account(mut, desc = "Fraction treasury")] | ||
| /// pub fraction_treasury: std::marker::PhantomData<()>, | ||
| /// | ||
| /// #[account(mut, desc = "Uninitialized vault account")] | ||
| /// pub vault: std::marker::PhantomData<()>, | ||
| /// | ||
| /// #[account(optional_signer, desc = "Authority on the vault")] | ||
| /// pub authority: std::marker::PhantomData<()>, | ||
| /// | ||
| /// #[account(desc = "Token program")] | ||
| /// pub token_program: std::marker::PhantomData<()>, | ||
| /// } | ||
| /// | ||
| /// // Then reference it in your instruction enum: | ||
| /// #[derive(ShankInstruction)] | ||
| /// pub enum VaultInstruction { | ||
| /// #[accounts(CreateVaultAccounts)] | ||
| /// InitVault(InitVaultArgs), | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// # Generated Code | ||
| /// | ||
| /// ShankAccounts generates: | ||
| /// 1. **IDL Metadata Methods** - For shank-idl to extract account information | ||
| /// 2. **Context Structs** - `{StructName}Context<'a>` with `AccountInfo<'a>` fields | ||
| /// 3. **Context Methods** - `{StructName}::context(program_id, accounts)` for validation | ||
| /// | ||
| /// # Usage in Solana Programs | ||
| /// | ||
| /// ```ignore | ||
| /// pub fn process_init_vault( | ||
| /// program_id: &Pubkey, | ||
| /// accounts: &[AccountInfo], | ||
| /// data: &[u8], | ||
| /// ) -> ProgramResult { | ||
| /// let ctx = CreateVaultAccounts::context(program_id, accounts)?; | ||
| /// | ||
| /// // Type-safe access by name: | ||
| /// msg!("Vault: {}", ctx.vault.key); | ||
| /// msg!("Authority: {}", ctx.authority.key); | ||
| /// | ||
| /// Ok(()) | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// Note: The field types don't affect IDL generation - ShankAccounts only processes | ||
| /// the `#[account(...)]` attributes. In real Solana programs, use `AccountInfo<'info>` | ||
| /// from `solana_program::account_info` for field types. | ||
| #[proc_macro_derive(ShankAccounts, attributes(account))] | ||
| pub fn shank_accounts(input: TokenStream) -> TokenStream { | ||
| let input = parse_macro_input!(input as DeriveInput); | ||
| derive_accounts(input) | ||
| .unwrap_or_else(to_compile_error) | ||
| .into() | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
ShankAccounts derive entrypoint and docs are solid; consider explicitly calling out #[accounts(StructName)] here
The ShankAccounts proc‑macro entrypoint correctly delegates to derive_accounts and uses to_compile_error for error propagation. The docs give a good overview and examples.
You might consider adding a short subsection explicitly documenting the #[accounts(StructName)] usage on ShankInstruction variants (currently only shown in the example block) so users can quickly see how ShankAccounts ties into instruction enums.
🤖 Prompt for AI Agents
In shank-macro/src/lib.rs around lines 361 to 453, the ShankAccounts docs show
an example but do not explicitly call out the #[accounts(StructName)] usage on
ShankInstruction variants; add a short subsection immediately after the existing
example that briefly explains how to reference the generated accounts struct
from an instruction enum using #[accounts(StructName)] (one-sentence
description) and include the minimal example line (#[derive(ShankInstruction)]
pub enum X { #[accounts(CreateVaultAccounts)] InitVault(InitVaultArgs), }) to
make the linkage explicit for readers.
| // Mock program ID | ||
| pub const ID: [u8; 32] = [1; 32]; | ||
|
|
||
| // Enable the solana-program feature for this example | ||
| #[cfg(feature = "solana-program")] | ||
| use solana_program::{ | ||
| account_info::AccountInfo, entrypoint::ProgramResult, msg, pubkey::Pubkey, | ||
| }; | ||
|
|
||
| // Mock AccountInfo when solana-program feature is not enabled | ||
| #[cfg(not(feature = "solana-program"))] | ||
| 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.
🧩 Analysis chain
Fix ID type under solana-program to match AccountInfo::key.
With feature = "solana-program", AccountInfo<'info>::key is &Pubkey, but ID is defined as [u8; 32]. The generated optional-account logic compares accounts[i].key == &crate::ID, so this will fail to compile in Solana builds.
Define ID so its type matches the key type in each cfg branch, e.g.:
-// Mock program ID
-pub const ID: [u8; 32] = [1; 32];
+// Program ID sentinel used for optional-account detection
+#[cfg(feature = "solana-program")]
+solana_program::declare_id!("ShankExample11111111111111111111111111111");
+
+#[cfg(not(feature = "solana-program"))]
+pub const ID: [u8; 32] = [1; 32];This keeps the mock [u8; 32] path for the non-Solana example while making crate::ID a Pubkey when using real solana_program::AccountInfo.
Please re-run cargo build --features solana-program --examples after this change to confirm the example and generated context() compile cleanly.
🏁 Script executed:
#!/bin/bash
# Find and examine the anchor_style_example.rs file
fd -t f "anchor_style_example.rs" --exec cat {} \;Length of output: 4392
🏁 Script executed:
#!/bin/bash
# Search for how optional accounts are handled in code generation
# Look for patterns that compare account keys with ID
rg -nA5 -B5 "optional.*account\|key.*ID\|Option.*AccountInfo" --type rs -g "!examples" 2>/dev/null | head -100Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there are any derive macros or code generation related to optional accounts
rg -n "ShankContext\|shank_context\|optional" --type rs -g "!examples" | head -30Length of output: 26
🏁 Script executed:
#!/bin/bash
# Look at the test or example files to see how ID is typically defined in the codebase
fd -t f -e "rs" -path "*/examples/*" --exec grep -l "ID.*32\|declare_id" {} \;Length of output: 168
🏁 Script executed:
#!/bin/bash
# Check cargo.toml to understand the solana-program feature
find . -name "Cargo.toml" -exec grep -l "solana-program" {} \; | head -5Length of output: 18
🌐 Web query:
solana_program::declare_id macro syntax usage
💡 Result:
Short answer
- declare_id! takes a single base58 string literal (the program pubkey) and expands to a static Pubkey + helpers (ID / id() etc.). Use it in your crate (usually lib.rs) to embed the program ID at compile time. [1][2][3]
Minimal example
- In lib.rs:
use solana_program::{declare_id, pubkey::Pubkey};
declare_id!("FILL_IN_YOUR_PROGRAM_PUBKEY_HERE");
// expands to a static ID and an id() helper you can call in code.
Notes
- The macro expects a literal (base58) string; it creates compile-time constants so you don’t need runtime parsing. [1][2]
- See discussion of purpose (defines static Pubkey) for usage rationale. [3]
Sources
[1] solana_program::declare_id docs.
[2] solana-sdk declare_id docs.
[3] Solana StackExchange — “What is the point of declare_id?”
---
🏁 Script executed:
```shell
#!/bin/bash
# Find shank macro/derive implementation files
find . -name "*.rs" -path "*/shank/*" ! -path "*/examples/*" ! -path "*/tests/*" | head -20
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Search for ShankAccounts and ShankContext implementations
find . -name "*.rs" -path "*/shank/*" ! -path "*/examples/*" ! -path "*/tests/*" -exec grep -l "ShankAccounts\|ShankContext\|derive.*Shank" {} \;Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for code generation logic related to optional accounts or ID comparison
find . -name "*.rs" -path "*/shank/*" ! -path "*/examples/*" ! -path "*/tests/*" -exec grep -l "optional\|Option\|accounts\[" {} \;Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check the structure of the shank directory
find ./shank -type f -name "*.rs" ! -path "*/target/*" ! -path "*/.git/*" | head -30Length of output: 1195
🏁 Script executed:
#!/bin/bash
# Search for the actual proc macro code or context generation
find . -name "*.rs" ! -path "*/target/*" -exec grep -l "fn context\|generate.*context\|optional.*account" {} \; | grep -v examples | head -10Length of output: 566
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine the context.rs implementation
cat ./shank/src/context.rs
Length of output: 171
🏁 Script executed:
#!/bin/bash
# Look at optional accounts test input and expanded output
echo "=== OPTIONAL ACCOUNTS INPUT ===" && cat ./shank/tests/expand/optional_accounts.rs && echo -e "\n=== OPTIONAL ACCOUNTS EXPANDED ===" && cat ./shank/tests/expand/optional_accounts.expanded.rsLength of output: 3793
🏁 Script executed:
#!/bin/bash
# Search for any ID-based validation or comparison in the codebase
find . -name "*.rs" ! -path "*/target/*" ! -path "*/examples/*" -exec grep -n "crate::ID\|== *ID\|&.*ID" {} + | head -20Length of output: 2618
🏁 Script executed:
#!/bin/bash
# Check the AccountInfo trait definition in solana_program (if available locally)
# or check if there's a mock definition showing the type difference
grep -rn "pub key:" ./shank --include="*.rs" | grep -v targetLength of output: 2347
🏁 Script executed:
#!/bin/bash
# Check Cargo.toml for feature definitions
cat ./shank/Cargo.tomlLength of output: 564
---
🌐 Web query:
```web
Solana Pubkey type definition bytes array
💡 Result:
Short answer: a Solana Pubkey is a 32‑byte value.
Details:
- In Rust (solana-program / solana-sdk) Pubkey is a wrapper around [u8; 32] (repr transparent); PUBKEY_BYTES = 32 and Pubkey::to_bytes() returns [u8; 32]. [1]
- In JavaScript (@solana/web3.js) PublicKey.toBytes() returns a Uint8Array of length 32. [2]
Sources:
[1] solana-program / solana-sdk pubkey (source/docs showing Pubkey([u8;32]), PUBKEY_BYTES, to_bytes).
[2] @solana/web3.js PublicKey API (toBytes() → Uint8Array).
Fix ID type under solana-program to match AccountInfo::key.
With feature = "solana-program", the generated context() method compares accounts[i].key == &crate::ID where key is &Pubkey (a transparent wrapper around [u8; 32]), but ID is defined as [u8; 32]. This type mismatch causes compilation failure.
Define ID so its type matches in each cfg branch:
-// Mock program ID
-pub const ID: [u8; 32] = [1; 32];
+// Program ID sentinel used for optional-account detection
+#[cfg(feature = "solana-program")]
+solana_program::declare_id!("ShankExample11111111111111111111111111111");
+
+#[cfg(not(feature = "solana-program"))]
+pub const ID: [u8; 32] = [1; 32];This keeps the mock [u8; 32] for non-Solana builds while making crate::ID a Pubkey when using real solana_program::AccountInfo.
🤖 Prompt for AI Agents
In shank/examples/anchor_style_example.rs around lines 5-20, the crate-level ID
is currently a [u8;32] causing a type mismatch with AccountInfo::key (which is
&Pubkey) when feature = "solana-program" is enabled; change the code to provide
two cfg branches: under #[cfg(feature = "solana-program")] define pub const ID:
solana_program::pubkey::Pubkey =
solana_program::pubkey::Pubkey::new_from_array([1; 32]) (or equivalent Pubkey
constructor), and keep the existing pub const ID: [u8; 32] = [1; 32] under
#[cfg(not(feature = "solana-program"))], so the ID type matches AccountInfo::key
in each build configuration.
| pub fn process_create_machine_v1<'info>( | ||
| accounts: &'info [AccountInfo<'info>], | ||
| name: String, | ||
| uri: String, | ||
| plugins: Vec<u8>, | ||
| ) -> ProgramResult { | ||
| // Use the generated context method for type-safe account access | ||
| let ctx = CreateMachineV1Accounts::context(accounts)?; | ||
|
|
||
| // Access accounts by name with compile-time guarantees | ||
| msg!("Creating machine: {}", name); | ||
| msg!("Machine account: {:?}", ctx.machine.key); | ||
| msg!("Collection: {:?}", ctx.machine_collection.key); | ||
| msg!("Owner: {:?}", ctx.owner.key); | ||
| msg!("Payer: {:?}", ctx.payer.key); | ||
|
|
||
| // Handle optional accounts safely | ||
| match ctx.authority { | ||
| Some(authority) => { | ||
| msg!("Authority provided: {:?}", authority.key); | ||
| } | ||
| None => { | ||
| msg!("No authority provided"); | ||
| } | ||
| } | ||
|
|
||
| // Machine creation logic would go here | ||
| msg!("Machine '{}' created successfully", name); | ||
|
|
||
| Ok(()) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider handling or silencing unused uri/plugins parameters.
In process_create_machine_v1, uri and plugins are never used, which may trigger warnings (or errors if #![deny(unused_variables)] is set). Either use them in logging or prefix with _ to make the intent explicit:
-pub fn process_create_machine_v1<'info>(
- accounts: &'info [AccountInfo<'info>],
- name: String,
- uri: String,
- plugins: Vec<u8>,
-) -> ProgramResult {
+pub fn process_create_machine_v1<'info>(
+ accounts: &'info [AccountInfo<'info>],
+ name: String,
+ _uri: String,
+ _plugins: Vec<u8>,
+) -> ProgramResult {If you intend to use uri/plugins in real logic, consider adding at least one log line referencing them so the example demonstrates their purpose.
🤖 Prompt for AI Agents
In shank/examples/anchor_style_example.rs around lines 84 to 113, the function
parameters `uri` and `plugins` are unused which can trigger compiler warnings;
either reference them (e.g., add a msg! logging line for `uri` and `plugins`) or
rename them to start with an underscore (e.g., `_uri`, `_plugins`) to explicitly
silence unused-variable warnings — pick logging if you plan to demonstrate their
use, otherwise rename to underscore-prefixed names.
| // Mock AccountInfo for testing | ||
| pub struct AccountInfo<'info> { | ||
| pub key: &'info [u8; 32], | ||
| pub data: &'info [u8], | ||
| pub owner: &'info [u8; 32], | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_conflicting_constraints_compile_time_error() { | ||
| // Note: These would cause compile-time errors, but we can't easily test | ||
| // compile-time failures in unit tests. However, we can document the expected behavior. | ||
|
|
||
| // This SHOULD cause an error (but we can't test it in a unit test): | ||
| // #[derive(ShankAccounts)] | ||
| // pub struct ConflictingAccounts<'info> { | ||
| // #[account(signer, optional_signer)] // Should error: can't be both | ||
| // pub bad_account: AccountInfo<'info>, | ||
| // } | ||
|
|
||
| // Instead, we test valid non-conflicting combinations | ||
| #[derive(ShankAccounts)] | ||
| pub struct ValidAccounts<'info> { | ||
| #[account(signer)] | ||
| pub signer_account: &'info AccountInfo<'info>, | ||
|
|
||
| #[account(optional_signer)] | ||
| pub optional_signer_account: Option<&'info AccountInfo<'info>>, | ||
|
|
||
| #[account(mut, signer)] // This combination is valid | ||
| pub mut_signer: &'info AccountInfo<'info>, | ||
| } | ||
|
|
||
| let metadata = ValidAccounts::__shank_accounts(); | ||
| assert_eq!(metadata.len(), 3); | ||
|
|
||
| // Verify signer | ||
| assert_eq!(metadata[0].3, true); // signer | ||
| assert_eq!(metadata[0].4, false); // not optional_signer | ||
|
|
||
| // Verify optional_signer | ||
| assert_eq!(metadata[1].3, false); // not regular signer | ||
| assert_eq!(metadata[1].4, true); // optional_signer | ||
|
|
||
| // Verify combined constraints work | ||
| assert_eq!(metadata[2].2, true); // writable | ||
| assert_eq!(metadata[2].3, true); // signer | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_description_variants() { | ||
| #[derive(ShankAccounts)] | ||
| pub struct DescriptionVariants<'info> { | ||
| #[account(desc = "Using desc")] | ||
| pub desc_account: &'info AccountInfo<'info>, | ||
|
|
||
| // Test that we properly handle all description attribute names | ||
| // (This tests our parsing logic) | ||
| pub no_desc_account: &'info AccountInfo<'info>, | ||
| } | ||
|
|
||
| let metadata = DescriptionVariants::__shank_accounts(); | ||
| assert_eq!(metadata.len(), 2); | ||
|
|
||
| // Check description is captured | ||
| assert_eq!(metadata[0].6, Some("Using desc".to_string())); | ||
|
|
||
| // Check no description | ||
| assert_eq!(metadata[1].6, None); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_field_order_preservation() { | ||
| #[derive(ShankAccounts)] | ||
| pub struct OrderedAccounts<'info> { | ||
| pub first: &'info AccountInfo<'info>, | ||
| pub second: &'info AccountInfo<'info>, | ||
| pub third: &'info AccountInfo<'info>, | ||
| pub fourth: &'info AccountInfo<'info>, | ||
| } | ||
|
|
||
| let metadata = OrderedAccounts::__shank_accounts(); | ||
| assert_eq!(metadata.len(), 4); | ||
|
|
||
| // Verify order is preserved | ||
| assert_eq!(metadata[0].0, 0); | ||
| assert_eq!(metadata[0].1, "first"); | ||
|
|
||
| assert_eq!(metadata[1].0, 1); | ||
| assert_eq!(metadata[1].1, "second"); | ||
|
|
||
| assert_eq!(metadata[2].0, 2); | ||
| assert_eq!(metadata[2].1, "third"); | ||
|
|
||
| assert_eq!(metadata[3].0, 3); | ||
| assert_eq!(metadata[3].1, "fourth"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_complex_constraint_combinations() { | ||
| #[derive(ShankAccounts)] | ||
| pub struct ComplexAccounts<'info> { | ||
| // All basic combinations that make sense | ||
| #[account(mut)] | ||
| pub mut_only: &'info AccountInfo<'info>, | ||
|
|
||
| #[account(signer)] | ||
| pub signer_only: &'info AccountInfo<'info>, | ||
|
|
||
| #[account(optional)] | ||
| pub optional_only: Option<&'info AccountInfo<'info>>, | ||
|
|
||
| #[account(optional_signer)] | ||
| pub optional_signer_only: Option<&'info AccountInfo<'info>>, | ||
|
|
||
| #[account(mut, signer)] | ||
| pub mut_signer: &'info AccountInfo<'info>, | ||
|
|
||
| #[account(mut, optional)] | ||
| pub mut_optional: Option<&'info AccountInfo<'info>>, | ||
|
|
||
| #[account(mut, signer, desc = "Complex account")] | ||
| pub mut_signer_desc: &'info AccountInfo<'info>, | ||
| } | ||
|
|
||
| let metadata = ComplexAccounts::__shank_accounts(); | ||
| assert_eq!(metadata.len(), 7); | ||
|
|
||
| // Test mut_only | ||
| assert!(metadata[0].2); // writable | ||
| assert!(!metadata[0].3); // not signer | ||
| assert!(!metadata[0].4); // not optional_signer | ||
| assert!(!metadata[0].5); // not optional | ||
|
|
||
| // Test signer_only | ||
| assert!(!metadata[1].2); // not writable | ||
| assert!(metadata[1].3); // signer | ||
| assert!(!metadata[1].4); // not optional_signer | ||
| assert!(!metadata[1].5); // not optional | ||
|
|
||
| // Test optional_only | ||
| assert!(!metadata[2].2); // not writable | ||
| assert!(!metadata[2].3); // not signer | ||
| assert!(!metadata[2].4); // not optional_signer | ||
| assert!(metadata[2].5); // optional | ||
|
|
||
| // Test optional_signer_only | ||
| assert!(!metadata[3].2); // not writable | ||
| assert!(!metadata[3].3); // not regular signer | ||
| assert!(metadata[3].4); // optional_signer | ||
| assert!(!metadata[3].5); // not optional (optional_signer is different) | ||
|
|
||
| // Test mut_signer combination | ||
| assert!(metadata[4].2); // writable | ||
| assert!(metadata[4].3); // signer | ||
| assert!(!metadata[4].4); // not optional_signer | ||
| assert!(!metadata[4].5); // not optional | ||
|
|
||
| // Test mut_optional combination | ||
| assert!(metadata[5].2); // writable | ||
| assert!(!metadata[5].3); // not signer | ||
| assert!(!metadata[5].4); // not optional_signer | ||
| assert!(metadata[5].5); // optional | ||
|
|
||
| // Test all combined with description | ||
| assert!(metadata[6].2); // writable | ||
| assert!(metadata[6].3); // signer | ||
| assert!(!metadata[6].4); // not optional_signer | ||
| assert!(!metadata[6].5); // not optional | ||
| assert_eq!(metadata[6].6, Some("Complex account".to_string())); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Comprehensive positive‑path coverage for ShankAccounts
The tests exercise signer vs optional_signer, mut, optional, descriptions, field order, and complex combinations, and they match the tuple layout produced by __shank_accounts(). This is a solid baseline for the new macro behavior.
A possible follow‑up (not blocking) would be to add compile‑fail tests (e.g., via trybuild) for misuses like conflicting attributes or non‑Option types on #[account(optional)] / #[account(optional_signer)], once the macro enforces those constraints.
🤖 Prompt for AI Agents
In shank/tests/error_handling_test.rs lines 6-175, the reviewer suggests adding
compile-fail tests for macro misuse; add a trybuild-based test suite that
includes .rs fixtures demonstrating invalid cases (e.g., conflicting attributes
signer+optional_signer, #[account(optional)] on non-Option types,
optional_signer on non-Option, etc.), add trybuild as a dev-dependency in
Cargo.toml, create a new test (e.g., shank/tests/compile_fail_tests.rs) that
invokes trybuild::TestCases::new().compile_fail() against the fixture files, and
place the failing fixtures under shank/tests/trybuild/*.rs so CI will assert the
macro emits compile errors for those misuse cases.
| /// Create a context from a slice of accounts | ||
| /// | ||
| /// This method parses the accounts according to the struct definition | ||
| /// and returns a Context containing the account struct. | ||
| /// | ||
| /// Optional accounts are determined by checking if the account key | ||
| /// equals the program ID (crate::ID). If so, they are set to None, otherwise Some. | ||
| pub fn context( |
There was a problem hiding this comment.
Fix doc comment: BasicAccounts has no optional accounts
The context docstring mentions optional-account handling via crate::ID, but BasicAccounts<'info> contains only required accounts. That part of the comment is misleading for this fixture; suggest dropping the optional-accounts paragraph:
- ///
- /// Optional accounts are determined by checking if the account key
- /// equals the program ID (crate::ID). If so, they are set to None, otherwise Some.🤖 Prompt for AI Agents
In shank/tests/expand/basic_accounts.expanded.rs around lines 56 to 63, the doc
comment for the context method erroneously describes optional-account handling
via checking for crate::ID, but BasicAccounts<'info> contains only required
accounts; remove the paragraph that mentions optional accounts (the lines
explaining crate::ID detection and setting Some/None) so the docstring only
describes creating a context from a slice of required accounts and parsing them
into the Context containing the account struct.
| // Verify all account structures work correctly | ||
| assert_eq!(BasicAccounts::__shank_accounts().len(), 1); | ||
| assert_eq!(CreateAccounts::__shank_accounts().len(), 3); | ||
| assert_eq!(UpdateAccounts::__shank_accounts().len(), 3); | ||
| assert_eq!(AdminAccounts::__shank_accounts().len(), 3); | ||
|
|
||
| // Verify specific constraints are applied correctly | ||
| let admin_accounts = AdminAccounts::__shank_accounts(); | ||
| assert_eq!(admin_accounts[0].1, "admin"); | ||
| assert_eq!(admin_accounts[0].3, true); // signer | ||
|
|
||
| assert_eq!(admin_accounts[1].1, "config"); | ||
| assert_eq!(admin_accounts[1].2, true); // mut | ||
|
|
||
| assert_eq!(admin_accounts[2].1, "co_signer"); | ||
| assert_eq!(admin_accounts[2].4, true); // optional_signer | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Fix optional_signer assertion to check the correct tuple fields.
For AdminAccounts, the test currently does:
assert_eq!(admin_accounts[2].1, "co_signer");
assert_eq!(admin_accounts[2].4, true); // optional_signerFrom the expanded tests, the tuple layout of __shank_accounts() is:
(index, name, is_mut, is_signer, is_pda, is_optional, desc)So:
optional_signershould setis_signer(.3) andis_optional(.5),.4is the PDA/seeds flag and will befalsehere.
Update the assertion to match the actual semantics:
- assert_eq!(admin_accounts[2].1, "co_signer");
- assert_eq!(admin_accounts[2].4, true); // optional_signer
+ assert_eq!(admin_accounts[2].1, "co_signer");
+ assert_eq!(admin_accounts[2].3, true); // signer
+ assert_eq!(admin_accounts[2].5, true); // optionalThis should align with the behavior seen in shank/tests/expand/complex_constraints.expanded.rs and optional_accounts.expanded.rs where optional accounts set the last boolean flag.
🏁 Script executed:
#!/bin/bash
# Find and examine the test file
echo "=== Test file content ==="
find . -name "instruction_integration_test.rs" -type f | head -5Length of output: 71
🏁 Script executed:
#!/bin/bash
# Look for the expanded test files referenced
echo "=== Searching for expanded test files ==="
find . -name "*expanded.rs" -type f | grep -E "(complex_constraints|optional_accounts)"Length of output: 145
🏁 Script executed:
#!/bin/bash
# Search for __shank_accounts definition or usage to understand tuple structure
echo "=== Searching for __shank_accounts usage ==="
rg -n "__shank_accounts" --type rs -C 5 | head -100Length of output: 71
🏁 Script executed:
#!/bin/bash
# View the test file at lines 195-211
echo "=== Test file lines 195-211 ==="
sed -n '195,211p' ./shank/tests/instruction_integration_test.rsLength of output: 755
🏁 Script executed:
#!/bin/bash
# View the optional_accounts expanded file
echo "=== optional_accounts.expanded.rs content ==="
cat ./shank/tests/expand/optional_accounts.expanded.rsLength of output: 3109
🏁 Script executed:
#!/bin/bash
# View the complex_constraints expanded file
echo "=== complex_constraints.expanded.rs content ==="
cat ./shank/tests/expand/complex_constraints.expanded.rsLength of output: 4443
🏁 Script executed:
#!/bin/bash
# Search for __shank_accounts usage with correct type
echo "=== Searching for __shank_accounts usage ==="
rg -n "__shank_accounts" --type rust -C 3Length of output: 29843
🏁 Script executed:
#!/bin/bash
# Find AdminAccounts definition in the test file
echo "=== Searching for AdminAccounts definition ==="
rg -n "struct AdminAccounts" ./shank/tests/instruction_integration_test.rs -A 10Length of output: 455
🏁 Script executed:
#!/bin/bash
# Look for the entire test function to see context
echo "=== Viewing the test function around line 180 ==="
sed -n '170,220p' ./shank/tests/instruction_integration_test.rsLength of output: 1708
Fix optional_signer assertion to check the correct tuple fields.
The test currently checks admin_accounts[2].4 for the optional_signer constraint, but the __shank_accounts() tuple layout is:
(index, name, is_mut, is_signer, is_pda, is_optional, desc)
with indices: (.0, .1, .2, .3, .4, .5, .6)
Since co_signer uses #[account(optional_signer)], it should set both .3 (is_signer) and .5 (is_optional) to true, while .4 (is_pda) remains false.
Update the assertion:
- assert_eq!(admin_accounts[2].1, "co_signer");
- assert_eq!(admin_accounts[2].4, true); // optional_signer
+ assert_eq!(admin_accounts[2].1, "co_signer");
+ assert_eq!(admin_accounts[2].3, true); // signer
+ assert_eq!(admin_accounts[2].5, true); // optional📝 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.
| // Verify all account structures work correctly | |
| assert_eq!(BasicAccounts::__shank_accounts().len(), 1); | |
| assert_eq!(CreateAccounts::__shank_accounts().len(), 3); | |
| assert_eq!(UpdateAccounts::__shank_accounts().len(), 3); | |
| assert_eq!(AdminAccounts::__shank_accounts().len(), 3); | |
| // Verify specific constraints are applied correctly | |
| let admin_accounts = AdminAccounts::__shank_accounts(); | |
| assert_eq!(admin_accounts[0].1, "admin"); | |
| assert_eq!(admin_accounts[0].3, true); // signer | |
| assert_eq!(admin_accounts[1].1, "config"); | |
| assert_eq!(admin_accounts[1].2, true); // mut | |
| assert_eq!(admin_accounts[2].1, "co_signer"); | |
| assert_eq!(admin_accounts[2].4, true); // optional_signer | |
| } | |
| // Verify all account structures work correctly | |
| assert_eq!(BasicAccounts::__shank_accounts().len(), 1); | |
| assert_eq!(CreateAccounts::__shank_accounts().len(), 3); | |
| assert_eq!(UpdateAccounts::__shank_accounts().len(), 3); | |
| assert_eq!(AdminAccounts::__shank_accounts().len(), 3); | |
| // Verify specific constraints are applied correctly | |
| let admin_accounts = AdminAccounts::__shank_accounts(); | |
| assert_eq!(admin_accounts[0].1, "admin"); | |
| assert_eq!(admin_accounts[0].3, true); // signer | |
| assert_eq!(admin_accounts[1].1, "config"); | |
| assert_eq!(admin_accounts[1].2, true); // mut | |
| assert_eq!(admin_accounts[2].1, "co_signer"); | |
| assert_eq!(admin_accounts[2].3, true); // signer | |
| assert_eq!(admin_accounts[2].5, true); // optional | |
| } |
🤖 Prompt for AI Agents
In shank/tests/instruction_integration_test.rs around lines 195 to 211, the test
incorrectly checks admin_accounts[2].4 for the optional_signer constraint;
update the assertions so that for the "co_signer" account you assert that .3
(is_signer) is true and .5 (is_optional) is true, and also assert that .4
(is_pda) is false to ensure the tuple fields match the documented layout.
| // Mock program ID - this simulates what declare_id! macro would create | ||
| pub const ID: [u8; 32] = [1; 32]; | ||
|
|
||
| // Mock AccountInfo for testing | ||
| 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.
🧹 Nitpick | 🔵 Trivial
Consider consolidating test mocks into a shared module.
The mock ID constant and AccountInfo struct are likely duplicated across multiple test files. Extracting them into a shared test utilities module (e.g., tests/common/mod.rs) would reduce duplication and improve maintainability.
🤖 Prompt for AI Agents
In shank/tests/simple_context_test.rs around lines 3 to 11, the test-local mock
ID constant and AccountInfo struct are duplicated across tests; extract these
into a shared test utilities module (e.g., create tests/common/mod.rs) and
replace the local definitions with re-exports or use statements. Move pub const
ID and pub struct AccountInfo<'info> into tests/common/mod.rs, update Cargo.toml
if needed to expose the tests module, and change simple_context_test.rs to
import the shared items (use crate::common::ID; use crate::common::AccountInfo;)
to eliminate duplication and centralize test mocks.
| #[test] | ||
| fn test_context_method() { | ||
| // Test that the IDL generation works | ||
| let accounts = SimpleAccounts::__shank_accounts(); | ||
| assert_eq!(accounts.len(), 3); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Enhance test assertions to verify account metadata.
The test only checks the length of the accounts vector. Consider asserting the actual account properties (names, mutability, signer flags, optional flags) to ensure the macro generates correct metadata.
Apply this diff:
#[test]
fn test_context_method() {
// Test that the IDL generation works
let accounts = SimpleAccounts::__shank_accounts();
assert_eq!(accounts.len(), 3);
+
+ // Verify payer account
+ assert_eq!(accounts[0].0, 0); // index
+ assert_eq!(accounts[0].1, "payer"); // name
+ assert_eq!(accounts[0].2, true); // writable
+ assert_eq!(accounts[0].3, true); // signer
+ assert_eq!(accounts[0].5, false); // not optional
+
+ // Verify data account
+ assert_eq!(accounts[1].0, 1);
+ assert_eq!(accounts[1].1, "data");
+ assert_eq!(accounts[1].2, true); // writable
+ assert_eq!(accounts[1].3, false); // not signer
+ assert_eq!(accounts[1].5, false); // not optional
+
+ // Verify optional_account
+ assert_eq!(accounts[2].0, 2);
+ assert_eq!(accounts[2].1, "optional_account");
+ assert_eq!(accounts[2].2, false); // not writable
+ assert_eq!(accounts[2].3, false); // not signer
+ assert_eq!(accounts[2].5, true); // optional
}📝 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_context_method() { | |
| // Test that the IDL generation works | |
| let accounts = SimpleAccounts::__shank_accounts(); | |
| assert_eq!(accounts.len(), 3); | |
| } | |
| #[test] | |
| fn test_context_method() { | |
| // Test that the IDL generation works | |
| let accounts = SimpleAccounts::__shank_accounts(); | |
| assert_eq!(accounts.len(), 3); | |
| // Verify payer account | |
| assert_eq!(accounts[0].0, 0); // index | |
| assert_eq!(accounts[0].1, "payer"); // name | |
| assert_eq!(accounts[0].2, true); // writable | |
| assert_eq!(accounts[0].3, true); // signer | |
| assert_eq!(accounts[0].5, false); // not optional | |
| // Verify data account | |
| assert_eq!(accounts[1].0, 1); | |
| assert_eq!(accounts[1].1, "data"); | |
| assert_eq!(accounts[1].2, true); // writable | |
| assert_eq!(accounts[1].3, false); // not signer | |
| assert_eq!(accounts[1].5, false); // not optional | |
| // Verify optional_account | |
| assert_eq!(accounts[2].0, 2); | |
| assert_eq!(accounts[2].1, "optional_account"); | |
| assert_eq!(accounts[2].2, false); // not writable | |
| assert_eq!(accounts[2].3, false); // not signer | |
| assert_eq!(accounts[2].5, true); // optional | |
| } |
🤖 Prompt for AI Agents
In shank/tests/simple_context_test.rs around lines 25 to 30, the test only
asserts the accounts vector length; update it to assert each account's metadata
as well: for every index (0..accounts.len()) assert the account's name,
mutability flag, signer flag, and optional flag match the expected values
produced by the macro (e.g., assert accounts[0].name == "<expected_name>";
assert accounts[0].is_mut == <expected_bool>; assert accounts[0].is_signer ==
<expected_bool>; assert accounts[0].is_optional == <expected_bool>), and repeat
for accounts[1] and accounts[2] to validate all generated metadata. Ensure the
expected values reflect the test fixture definitions.
1 participant
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.