-
Notifications
You must be signed in to change notification settings - Fork 19
✨ Components & Systems Bundle #207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
e5e786e to
5a95803
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (1)
-
clients/csharp/Solana.Unity.Bolt.Test/ECSTest.cs, line 186 (link)style: Inconsistent argument serialization - this method uses a plain object while line 166 uses Bolt.World.SerializeArgs(). Should be consistent across all ApplySystem calls.
76 files reviewed, 22 comments
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/ApplySystem.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fews suggestions by @GabrielePicco
5a95803 to
58a17a2
Compare
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughConsolidates attribute macros into a shared bolt-lang/attribute crate, removes bolt-component and bolt-system crates/manifests, adds bundle primitives and discriminator-based CPI flows in World, introduces Component/System/Identifier domain types in TS/C# clients, adds CLI bundle scaffolding and example-bundle, and updates CI/publish lists. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant SDK as Client SDK (TS/C#)
participant World as World Program
Note over Client,World: Bundle/system apply flow (named vs legacy)
Client->>SDK: Build Component/System domain objects
SDK->>SDK: For each (entity, components, seeds): compute component.pda(entity, seed)
SDK->>SDK: system_discriminator = system.getMethodDiscriminator("bolt_execute")
Client->>SDK: Bolt.World.ApplySystem(world, system, entities[], args, authority)
SDK->>World: Instruction (program=World, data = [maybe system_discriminator || legacy opcode] + args, accounts = [entity, component_pdas..., authority, ...])
alt system has discriminator (named)
World->>World: apply_with_discriminator(system_discriminator, args)
else legacy unnamed system
World->>World: apply(args)
end
World-->>SDK: Instruction processed / return data
SDK-->>Client: Tx submitted / result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 13
♻️ Duplicate comments (19)
examples/bundle/Cargo.toml (2)
25-25: Consider using workspace inheritance for serde.For consistency with other examples and easier dependency management, use workspace inheritance for the
serdedependency.If you decide to adopt this pattern, you would need to:
- Add serde to workspace dependencies in the root
Cargo.toml.- Apply this diff:
-serde = { version = "1.0", features = ["derive"] } +serde.workspace = trueThen update the root
Cargo.toml:serde = { version = "^1", features = ["derive"] }
3-3: Fix the version inconsistency and use workspace inheritance.The hardcoded version "0.2.5" is inconsistent with the workspace version "0.2.6" defined in the root
Cargo.toml. This can lead to version mismatches and maintenance issues.Apply this diff to use workspace inheritance:
-version = "0.2.5" +version = { workspace = true }clients/csharp/Solana.Unity.Bolt/ECS/Identifier.cs (1)
12-17: Prefer immutable properties to prevent unintended modification.The
ProgramandNameproperties have public setters, allowing external code to modify the identifier after construction. Consider making them{ get; private set; }or readonly to prevent accidental mutations.Apply this diff to improve immutability:
- public PublicKey Program { get; set; } + public PublicKey Program { get; private set; } - public string Name { get; set; } + public string Name { get; private set; }crates/bolt-lang/attribute/bundle/Cargo.toml (1)
2-3: The package name and description are redundant.The description "bolt-attribute-bolt-bundle" simply repeats the package name without providing meaningful information about what the crate does.
Consider using a descriptive string like "Procedural macro for bundling Bolt components and systems" as suggested in the previous review.
clients/typescript/test/low-level/ecs.ts (1)
13-13: Remove unused crypto import.The
cryptoimport is not used in this file. TheGetDiscriminatorfunction is imported from the library instead.Apply this diff:
-import crypto from "crypto";clients/typescript/src/index.ts (1)
119-119: Redundant PublicKey constructor.The
componentIdparameter is already aPublicKey, so wrapping it innew PublicKey()is unnecessary.Apply this diff:
- const programKey = new PublicKey(componentId); + const programKey = componentId;crates/bolt-cli/src/bundle.rs (1)
6-6: Incorrect comment.The comment incorrectly states "Create a new component" when it should say "Create a new bundle".
Apply this diff:
-// Create a new component from the template +// Create a new bundle from the templatecrates/bolt-lang/src/instructions/initialize.rs (1)
6-6: Remove unnecessaryClonebound.Line 6:
Account::set_innerconsumes the value, so addingT: Clonejust blocks non-Clonecomponents without providing any benefit. Please drop this constraint unless another caller genuinely relies on it.crates/bolt-lang/attribute/src/common/mod.rs (1)
18-31: Avoid cloning and usingmapfor side effects; mutate in-place.You’re cloning the entire items vec and returning a value that’s ignored. Use
if letand drop the clone.- module.content.as_mut().map(|(brace, items)| { - items.insert(0, syn::Item::Use(syn::parse_quote! { use super::*; })); - items.insert( - 1, - syn::Item::Struct(syn::parse_quote! { - #[derive(Accounts)] - pub struct VariadicBoltComponents<'info> { - #[account()] - pub authority: AccountInfo<'info>, - } - }), - ); - (brace, items.clone()) - }); + if let Some((_brace, items)) = module.content.as_mut() { + items.insert(0, syn::Item::Use(syn::parse_quote! { use super::*; })); + items.insert( + 1, + syn::Item::Struct(syn::parse_quote! { + #[derive(Accounts)] + pub struct VariadicBoltComponents<'info> { + #[account()] + pub authority: AccountInfo<'info>, + } + }), + ); + }clients/csharp/Solana.Unity.Bolt.Test/ECSTest.cs (2)
111-129: Use structured deserialization for Velocity (avoid brittle manual parsing).Mirror Position’s approach for maintainability.
- var accountInfo = await framework.GetAccountInfo(framework.BundleVelocityEntity1Pda); - var data = Convert.FromBase64String(accountInfo.Data[0]); - int offset = 8; // skip discriminator - long x = BitConverter.ToInt64(data, offset); offset += 8; - long y = BitConverter.ToInt64(data, offset); offset += 8; - long z = BitConverter.ToInt64(data, offset); offset += 8; - Debug.Assert(1 == x, "X is not equal to 1"); - Debug.Assert(2 == y, "Y is not equal to 2"); - Debug.Assert(3 == z, "Z is not equal to 3"); + var accountInfo = await framework.GetAccountInfo(framework.BundleVelocityEntity1Pda); + var data = Convert.FromBase64String(accountInfo.Data[0]); + var velocity = Velocity.Accounts.Velocity.Deserialize(data); + Debug.Assert(1 == velocity.X, "X is not equal to 1"); + Debug.Assert(2 == velocity.Y, "Y is not equal to 2"); + Debug.Assert(3 == velocity.Z, "Z is not equal to 3");If a Velocity model isn’t generated, prefer explicit little‑endian reads (BinaryPrimitives.ReadInt64LittleEndian).
222-245: Use structured deserialization for Velocity (same as above).Replace manual parsing with the generated Velocity model.
- var accountInfo = await framework.GetAccountInfo(framework.BundleVelocityEntity1Pda); - var data = Convert.FromBase64String(accountInfo.Data[0]); - int offset = 8; // skip discriminator - long x = BitConverter.ToInt64(data, offset); offset += 8; - long y = BitConverter.ToInt64(data, offset); offset += 8; - long z = BitConverter.ToInt64(data, offset); offset += 8; - Debug.Assert(0 == x, "X is not equal to 0"); - Debug.Assert(0 == y, "Y is not equal to 0"); - Debug.Assert(0 == z, "Z is not equal to 0"); + var accountInfo = await framework.GetAccountInfo(framework.BundleVelocityEntity1Pda); + var data = Convert.FromBase64String(accountInfo.Data[0]); + var velocity = Velocity.Accounts.Velocity.Deserialize(data); + Debug.Assert(0 == velocity.X, "X is not equal to 0"); + Debug.Assert(0 == velocity.Y, "Y is not equal to 0"); + Debug.Assert(0 == velocity.Z, "Z is not equal to 0");examples/bundle/src/lib.rs (2)
37-44: Align arg naming with other examples (_argsinstead of_args_p).- pub fn execute(ctx: Context<Components>, _args_p: Vec<u8>) -> Result<Components> { + pub fn execute(ctx: Context<Components>, _args: Vec<u8>) -> Result<Components> {
55-60: Align arg naming with other examples (_argsinstead of_args_p).- pub fn execute(ctx: Context<Components>, _args_p: Vec<u8>) -> Result<Components> { + pub fn execute(ctx: Context<Components>, _args: Vec<u8>) -> Result<Components> {crates/bolt-lang/src/instructions/update.rs (1)
33-78: Avoid cloning session_token/authority in validity context.Reuse references to reduce copies if ValidityChecker can accept refs; otherwise consider lightweight handles.
- let validity_ctx = crate::session_keys::ValidityChecker { - session_token: session_token.clone(), - session_signer: authority.clone(), + let validity_ctx = crate::session_keys::ValidityChecker { + session_token, + session_signer: authority, authority: component_authority, target_program: world::id_const(), };If ValidityChecker requires owned values, consider changing its fields to references.
crates/bolt-lang/attribute/src/component/generate/program.rs (2)
34-40: Replace.unwrap()with.expect()for clearer error messages.Using
.unwrap()in proc macro code generation can cause unhelpful compiler errors. When parsing fails, developers need descriptive messages to understand what went wrong.Apply this diff:
items.extend( vec![initialize_fn, initialize_struct, destroy_fn, destroy_struct] .into_iter() - .map(|item| syn::parse2(item).unwrap()) + .map(|item| syn::parse2(item).expect("Failed to parse generated component instruction item")) .collect::<Vec<_>>(), );
233-245: Replace.unwrap()with.expect()for clearer error messages.Using
.unwrap()in proc macro code generation can cause unhelpful compiler errors. When parsing fails, developers need descriptive messages to understand what went wrong.Apply this diff:
items.extend( vec![ update_fn, update_struct, update_with_session_fn, update_with_session_struct, ] .into_iter() - .map(|item| syn::parse2(item).unwrap()) + .map(|item| syn::parse2(item).expect("Failed to parse generated update instruction item")) .collect::<Vec<_>>(), );crates/bolt-lang/attribute/src/system/mod.rs (3)
296-304: Replace.unwrap()with.expect()for clearer error messages.Using
.unwrap()in proc macro code generation can cause unhelpful compiler errors.Apply this diff:
let initialize_extra_accounts = quote! { #[automatically_derived] pub fn init_extra_accounts(_ctx: Context<#struct_name>) -> Result<()> { Ok(()) } }; - content.push(syn::parse2(initialize_extra_accounts).unwrap()); + content.push(syn::parse2(initialize_extra_accounts).expect("Failed to parse init_extra_accounts function"));
324-380: Use defensive programming for path segment access.At line 341,
.unwrap()assumes the path has segments, but this could panic if the type structure is unexpected during macro expansion.Apply this diff to handle the None case gracefully:
if let Some(components_ty) = components_ty_opt { let components_with_info: Type = match components_ty { Type::Path(mut tp) => { - let seg = tp.path.segments.last_mut().unwrap(); + let seg = tp.path.segments.last_mut() + .expect("Type path must have at least one segment"); match &mut seg.arguments {
486-517: Use defensive programming for path segment access.At line 491,
.expect()would provide a clearer error message than.unwrap()if the Context type path has no segments.Apply this diff:
if let Type::Path(type_path) = &*pat_type.ty { - let last_segment = type_path.path.segments.last().expect("Context segment not found"); + let last_segment = type_path.path.segments.last() + .expect("Context type path must have at least one segment"); if last_segment.ident == "Context" {
🧹 Nitpick comments (11)
clients/typescript/test/low-level/permissioning/world.ts (1)
2-7: Verify GetDiscriminator import usage.
GetDiscriminatoris imported but not used in the visible code. If it's not used elsewhere in this file or intended for future changes, consider removing the unused import to keep the code clean.Run the following script to verify usage:
#!/bin/bash # Description: Check if GetDiscriminator is used in this file rg -n 'GetDiscriminator' clients/typescript/test/low-level/permissioning/world.tsclients/typescript/test/low-level/permissioning/component.ts (1)
8-8: Document discriminator naming convention.The pattern
scope:action(e.g.,global:initialize,global:destroy) is used consistently; please add or update documentation describing this convention to prevent future mismatches.crates/bolt-lang/src/cpi/mod.rs (1)
5-9: Consider more specific error messages.The function conflates two distinct failure modes (missing signature and incorrect address) into a single
InvalidCallererror. While functionally correct, this makes debugging harder.Consider separate error variants for clarity:
pub fn check(cpi_auth: &AccountInfo<'_>) -> Result<()> { - if !cpi_auth.is_signer || *cpi_auth.key != crate::world::World::cpi_auth_address() { + if !cpi_auth.is_signer { + return Err(BoltError::MissingSignature.into()); + } + if *cpi_auth.key != crate::world::World::cpi_auth_address() { return Err(BoltError::InvalidCaller.into()); } Ok(()) }Note: This requires adding a
MissingSignaturevariant toBoltError.clients/typescript/test/low-level/session.ts (1)
71-71: Consider extracting the magic string to a constant.The discriminator string
"global:initialize"appears multiple times. Magic strings are error-prone and harder to maintain.Consider defining a constant at the module or package level:
const GLOBAL_INITIALIZE_DISCRIMINATOR = GetDiscriminator("global:initialize");Then use it at both call sites:
- .initializeComponent(GetDiscriminator("global:initialize")) + .initializeComponent(GLOBAL_INITIALIZE_DISCRIMINATOR)This improves maintainability and reduces the risk of typos.
Also applies to: 160-160
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt.cs (1)
30-40: Add null check for the name parameter.The discriminator computation is correct and matches Anchor's approach. However, the method doesn't validate the
nameparameter.Add a null check at the beginning of the method:
public static byte[] GetDiscriminator(string name) { + if (name == null) { + throw new ArgumentNullException(nameof(name)); + } // Anchor uses the first 8 bytes of the SHA256 hash of the instruction name. // See: https://github.com/coral-xyz/anchor/blob/master/lang/syn/src/codegen/accounts/discriminator.rs var nameBytes = Encoding.UTF8.GetBytes(name);clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/ApplySystem.cs (1)
72-85: Guard against nullentitiesto avoid NRE.Iterate safely when no system inputs are required.
- foreach (var entry in entities) + foreach (var entry in (entities ?? Array.Empty<(PublicKey entity, Component[] components, string[] seeds)?>())) { if (entry == null) continue; var (entity, components, seeds) = entry.Value; for (int i = 0; i < components.Length; i++) { var comp = components[i]; var providedSeed = (seeds != null && i < seeds.Length) ? seeds[i] : ""; var pda = WorldProgram.FindComponentPda(comp.Program, entity, comp.Seeds(providedSeed)); remainingAccounts.Add(Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(comp.Program, false)); remainingAccounts.Add(Solana.Unity.Rpc.Models.AccountMeta.Writable(pda, false)); } }crates/bolt-lang/attribute/src/component/attributes.rs (3)
13-21: Avoid unwrap in proc-macro parsing.Panic on invalid attribute degrades DX. Use fallible parsing with a helpful error, or at least expect(...) with context.
- .map(|attr| Self::from(attr.parse_meta().unwrap())) + .map(|attr| attr.parse_meta().map(Self::from) + .expect("Invalid #[component(...)] attribute"))
23-33: Prefer graceful error for TokenStream parsing.Avoid panicking; bubble an error up to compile_error! or add context.
- attr.is_empty() + attr.is_empty() .not() .then(|| { - let attr_meta: Meta = syn::parse(attr).expect("Invalid component attribute"); + let attr_meta: Meta = syn::parse(attr).expect("Invalid #[component(...)] attribute"); Self::from(attr_meta) })
70-82: Delegate detection missesdelegate = trueform.Support NameValue boolean to be resilient.
- match meta { + match meta { Meta::Path(path) => path.is_ident("delegate"), - Meta::List(meta_list) => meta_list.nested.iter().any(|nested_meta| { + Meta::List(meta_list) => meta_list.nested.iter().any(|nested_meta| { if let NestedMeta::Meta(Meta::Path(path)) = nested_meta { path.is_ident("delegate") + } else if let NestedMeta::Meta(Meta::NameValue(nv)) = nested_meta { + nv.path.is_ident("delegate") && matches!(nv.lit, Lit::Bool(b) if b.value) } else { false } }), _ => false, }clients/csharp/Solana.Unity.Bolt/WorldProgram/World.cs (1)
256-277: Avoid fixed 1200-byte buffers; allocate exact size and de-duplicate encoding.Prevent overflow and reduce allocations; centralize encoding.
- byte[] _data = new byte[1200]; - int offset = 0; - _data.WriteSpan(IX_APPLY, offset); - offset += 8; - _data.WriteS32(args.Length, offset); - offset += 4; - _data.WriteSpan(args, offset); - offset += args.Length; - byte[] resultData = new byte[offset]; - Array.Copy(_data, resultData, offset); + var resultData = BuildApplyData(IX_APPLY, null, args); return new Solana.Unity.Rpc.Models.TransactionInstruction{Keys = keys, ProgramId = programId.KeyBytes, Data = resultData};- byte[] _data = new byte[1200]; - int offset = 0; - _data.WriteSpan(IX_APPLY_WITH_DISCRIMINATOR, offset); - offset += 8; - _data.WriteS32(system_discriminator.Length, offset); - offset += 4; - _data.WriteSpan(system_discriminator, offset); - offset += system_discriminator.Length; - _data.WriteS32(args.Length, offset); - offset += 4; - _data.WriteSpan(args, offset); - offset += args.Length; - byte[] resultData = new byte[offset]; - Array.Copy(_data, resultData, offset); + var resultData = BuildApplyData(IX_APPLY_WITH_DISCRIMINATOR, system_discriminator, args); return new Solana.Unity.Rpc.Models.TransactionInstruction{Keys = keys, ProgramId = programId.KeyBytes, Data = resultData};- byte[] _data = new byte[1200]; - int offset = 0; - _data.WriteSpan(IX_APPLY_WITH_SESSION, offset); - offset += 8; - _data.WriteS32(args.Length, offset); - offset += 4; - _data.WriteSpan(args, offset); - offset += args.Length; - byte[] resultData = new byte[offset]; - Array.Copy(_data, resultData, offset); + var resultData = BuildApplyData(IX_APPLY_WITH_SESSION, null, args); return new Solana.Unity.Rpc.Models.TransactionInstruction{Keys = keys, ProgramId = programId.KeyBytes, Data = resultData};- byte[] _data = new byte[1200]; - int offset = 0; - _data.WriteSpan(IX_APPLY_WITH_SESSION_AND_DISCRIMINATOR, offset); - offset += 8; - _data.WriteS32(system_discriminator.Length, offset); - offset += 4; - _data.WriteSpan(system_discriminator, offset); - offset += system_discriminator.Length; - _data.WriteS32(args.Length, offset); - offset += 4; - _data.WriteSpan(args, offset); - offset += args.Length; - byte[] resultData = new byte[offset]; - Array.Copy(_data, resultData, offset); + var resultData = BuildApplyData(IX_APPLY_WITH_SESSION_AND_DISCRIMINATOR, system_discriminator, args); return new Solana.Unity.Rpc.Models.TransactionInstruction{Keys = keys, ProgramId = programId.KeyBytes, Data = resultData};Add once in this file:
private static byte[] BuildApplyData(byte[] ixDisc, byte[] systemDisc, byte[] args) { var len = 8 + (systemDisc is null ? 0 : 4 + systemDisc.Length) + 4 + args.Length; var data = new byte[len]; var offset = 0; data.WriteSpan(ixDisc, offset); offset += 8; if (systemDisc is not null) { data.WriteS32(systemDisc.Length, offset); offset += 4; data.WriteSpan(systemDisc, offset); offset += systemDisc.Length; } data.WriteS32(args.Length, offset); offset += 4; data.WriteSpan(args, offset); return data; }Also applies to: 279-304, 306-328, 330-356
clients/typescript/src/world/transactions.ts (1)
575-578: Update interface to reflect domain object support.The
ApplySystemComponentinterface still declarescomponentId: PublicKey, but the implementation at line 482 wraps it withComponent.from(), indicating it acceptsPublicKey | Component.Apply this diff to align the interface with actual usage:
interface ApplySystemComponent { - componentId: PublicKey; + componentId: PublicKey | Component; seed?: string; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockclients/typescript/src/generated/idl/world.jsonis excluded by!**/generated/**clients/typescript/src/generated/types/world.tsis excluded by!**/generated/**
📒 Files selected for processing (79)
.github/workflows/publish-bolt-crates.yml(1 hunks)Anchor.toml(2 hunks)Cargo.toml(2 hunks)clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs(2 hunks)clients/csharp/Solana.Unity.Bolt.Test/ECSTest.cs(5 hunks)clients/csharp/Solana.Unity.Bolt.Test/Framework.cs(2 hunks)clients/csharp/Solana.Unity.Bolt/ECS/Component.cs(1 hunks)clients/csharp/Solana.Unity.Bolt/ECS/Identifier.cs(1 hunks)clients/csharp/Solana.Unity.Bolt/ECS/System.cs(1 hunks)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt.cs(2 hunks)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/ApplySystem.cs(2 hunks)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.cs(1 hunks)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DestroyComponent.cs(1 hunks)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/InitializeComponent.cs(1 hunks)clients/csharp/Solana.Unity.Bolt/WorldProgram/Generated.cs(1 hunks)clients/csharp/Solana.Unity.Bolt/WorldProgram/World.cs(5 hunks)clients/typescript/src/delegation/delegate.ts(3 hunks)clients/typescript/src/ecs/component.ts(1 hunks)clients/typescript/src/ecs/identifier.ts(1 hunks)clients/typescript/src/ecs/index.ts(1 hunks)clients/typescript/src/ecs/system.ts(1 hunks)clients/typescript/src/index.ts(3 hunks)clients/typescript/src/world/transactions.ts(10 hunks)clients/typescript/test/framework.ts(2 hunks)clients/typescript/test/intermediate-level/acceleration.ts(3 hunks)clients/typescript/test/intermediate-level/ecs.ts(4 hunks)clients/typescript/test/low-level/ecs.ts(6 hunks)clients/typescript/test/low-level/permissioning/component.ts(2 hunks)clients/typescript/test/low-level/permissioning/world.ts(1 hunks)clients/typescript/test/low-level/session.ts(3 hunks)crates/bolt-cli/src/bundle.rs(1 hunks)crates/bolt-cli/src/lib.rs(5 hunks)crates/bolt-cli/src/rust_template.rs(2 hunks)crates/bolt-cli/src/templates/bundle/lib.rs.template(1 hunks)crates/bolt-cli/src/templates/bundle/mod.rs(1 hunks)crates/bolt-cli/src/templates/mod.rs(1 hunks)crates/bolt-cli/src/templates/workspace/workspace.toml.template(1 hunks)crates/bolt-lang/Cargo.toml(1 hunks)crates/bolt-lang/attribute/Cargo.toml(1 hunks)crates/bolt-lang/attribute/bolt-program/src/lib.rs(0 hunks)crates/bolt-lang/attribute/bundle/Cargo.toml(2 hunks)crates/bolt-lang/attribute/bundle/src/lib.rs(1 hunks)crates/bolt-lang/attribute/component-deserialize/Cargo.toml(1 hunks)crates/bolt-lang/attribute/component-deserialize/src/lib.rs(3 hunks)crates/bolt-lang/attribute/component/Cargo.toml(1 hunks)crates/bolt-lang/attribute/component/src/lib.rs(1 hunks)crates/bolt-lang/attribute/delegate/src/lib.rs(0 hunks)crates/bolt-lang/attribute/extra-accounts/src/lib.rs(1 hunks)crates/bolt-lang/attribute/src/bundle/mod.rs(1 hunks)crates/bolt-lang/attribute/src/common/mod.rs(1 hunks)crates/bolt-lang/attribute/src/component/attributes.rs(1 hunks)crates/bolt-lang/attribute/src/component/generate/mod.rs(1 hunks)crates/bolt-lang/attribute/src/component/generate/program.rs(1 hunks)crates/bolt-lang/attribute/src/component/mod.rs(1 hunks)crates/bolt-lang/attribute/src/delegate/mod.rs(1 hunks)crates/bolt-lang/attribute/src/lib.rs(1 hunks)crates/bolt-lang/attribute/src/system/mod.rs(1 hunks)crates/bolt-lang/attribute/system-input/src/lib.rs(1 hunks)crates/bolt-lang/attribute/system/Cargo.toml(1 hunks)crates/bolt-lang/attribute/system/src/lib.rs(1 hunks)crates/bolt-lang/src/cpi/mod.rs(1 hunks)crates/bolt-lang/src/errors.rs(1 hunks)crates/bolt-lang/src/instructions/destroy.rs(1 hunks)crates/bolt-lang/src/instructions/initialize.rs(1 hunks)crates/bolt-lang/src/instructions/mod.rs(1 hunks)crates/bolt-lang/src/instructions/update.rs(1 hunks)crates/bolt-lang/src/lib.rs(3 hunks)crates/bolt-lang/utils/src/lib.rs(2 hunks)crates/programs/bolt-component/Cargo.toml(0 hunks)crates/programs/bolt-component/src/lib.rs(0 hunks)crates/programs/bolt-system/Cargo.toml(0 hunks)crates/programs/bolt-system/Xargo.toml(0 hunks)crates/programs/bolt-system/src/lib.rs(0 hunks)crates/programs/world/Cargo.toml(1 hunks)crates/programs/world/src/lib.rs(5 hunks)docs/REPORT.md(1 hunks)examples/bundle/Cargo.toml(1 hunks)examples/bundle/src/lib.rs(1 hunks)scripts/test-publish.sh(1 hunks)
💤 Files with no reviewable changes (7)
- crates/bolt-lang/attribute/bolt-program/src/lib.rs
- crates/programs/bolt-system/src/lib.rs
- crates/programs/bolt-system/Xargo.toml
- crates/bolt-lang/attribute/delegate/src/lib.rs
- crates/programs/bolt-component/src/lib.rs
- crates/programs/bolt-component/Cargo.toml
- crates/programs/bolt-system/Cargo.toml
🔇 Additional comments (75)
docs/REPORT.md (1)
7-8: Data update looks consistentBar values stay within the declared CU range and remain monotonic, so the visualization keeps its integrity.
crates/bolt-lang/utils/src/lib.rs (2)
5-5: LGTM: Variable rename improves clarity.Renaming
authority_fieldtobolt_metadata_fieldaligns the variable name with the actual field identifier, improving code readability.
19-19: Confirm no serialization break due to field reordering
Search didn’t reveal any directBorshSerialize/Serializederives referencingbolt_metadata, but please verify:
- The structs use name-based (not positional) serialization
- No existing on-chain accounts depend on the old field order
- All consumers of
add_bolt_metadatatolerate the new field positioncrates/bolt-lang/attribute/component-deserialize/Cargo.toml (1)
19-19: LGTM!The sha2 dependency addition aligns with the PR's discriminator-based routing feature (SHA-256 hashing of method names) and correctly uses workspace inheritance.
crates/bolt-cli/src/templates/workspace/workspace.toml.template (1)
6-6: LGTM!The addition of the
"programs-ecs/bundles/*"pattern to workspace members correctly supports the new bundling feature and is consistent with the existing patterns for components and systems.Cargo.toml (2)
22-23: LGTM!The addition of
bolt-attributeandbolt-attribute-bolt-bundleas workspace dependencies correctly supports the new bundling architecture. Version pinning is consistent with other workspace packages.
64-65: LGTM!The
const-cryptoandsha2dependencies are correctly added to support discriminator generation (SHA-256 hashing) as described in the PR objectives.crates/bolt-lang/attribute/extra-accounts/src/lib.rs (2)
74-75: LGTM!The explicit qualification of
NUMBER_OF_COMPONENTSthrough the trait bound is correct. The previous code would not compile becauseContext<T>doesn't directly have this associated constant—it must be accessed throughT's implementation ofNumberOfComponents.
80-80: LGTM!The addition of the
bolt_lang::NumberOfComponentstrait bound is necessary to support the qualified path used in line 74 for accessingNUMBER_OF_COMPONENTS.crates/programs/world/Cargo.toml (1)
30-30: LGTM!The addition of
const-cryptoas a workspace dependency is correct and supports the discriminator generation functionality needed by the world program for the new bundling architecture.Anchor.toml (2)
13-13: LGTM!The addition of
example-bundleto the localnet programs correctly demonstrates the new bundling feature introduced in this PR.
38-38: LGTM!The workspace members list is correctly updated to include
examples/bundleand remove the deprecatedbolt-componentandbolt-systemprograms, aligning with the PR's architectural changes..github/workflows/publish-bolt-crates.yml (1)
184-184: Ensurebolt-attribute.workspaceis published before the bundle
bolt-attribute-bolt-bundledirectly depends onbolt-attribute.workspace; confirm that-p bolt-attribute.workspaceprecedes-p bolt-attribute-bolt-bundlein.github/workflows/publish-bolt-crates.yml.scripts/test-publish.sh (1)
17-17: LGTM!The addition of
bolt-attribute-bolt-bundleto the publish list aligns with the PR's introduction of bundling support and the broader workspace restructuring.crates/bolt-lang/attribute/Cargo.toml (1)
2-16: LGTM!The package rename from
bolt-attribute-bolt-programtobolt-attributeand the addition ofbolt-utilsandheckdependencies align with the PR's goal to centralize attribute processing logic into a shared crate.clients/typescript/src/delegation/delegate.ts (1)
167-181: LGTM!The introduction of the
Componentwrapper and support forPublicKey | Componentparameter types is well-implemented. TheComponent.from()factory method correctly handles both input types, and the derivation ofownerProgramandcomponentPdafrom the component wrapper is appropriate.clients/csharp/Solana.Unity.Bolt.Test/Framework.cs (1)
59-75: LGTM!The addition of bundle-related properties (
ExampleBundleProgramId,BundlePositionEntity1Pda,BundleVelocityEntity1Pda) provides the necessary test infrastructure for validating bundled component workflows. The initialization pattern is consistent with existing framework properties.clients/csharp/Solana.Unity.Bolt/ECS/Identifier.cs (1)
33-37: LGTM!The
GetMethodDiscriminatorimplementation correctly builds the discriminator with the"global:"prefix and optional name prefix, mirroring the TypeScript implementation.crates/bolt-lang/attribute/component/Cargo.toml (1)
15-15: LGTM!Consolidating dependencies through
bolt-attributealigns with the PR's goal to centralize attribute processing logic and reduces duplication across attribute crates.clients/csharp/Solana.Unity.Bolt/ECS/Component.cs (2)
20-25: LGTM!The
Fromfactory method correctly handles bothComponentandPublicKeyinputs, with appropriate type checking and error handling for invalid input types.
30-33: LGTM!The
Seedsmethod correctly concatenates the seed and name with proper null handling, mirroring the TypeScript implementation.clients/typescript/test/intermediate-level/acceleration.ts (3)
47-71: LGTM!The test correctly creates a bundled component using the new
Componentwrapper with program ID and component name, and validates the initial state (x, y, z = 0).
93-114: LGTM!The bundled component delegation test correctly uses the
Componentwrapper and verifies that the delegated account owner matches theDELEGATION_PROGRAM_ID.
116-150: CI skip guard is appropriate for performance test.The test applies a system 10 times with 50ms delays, which would be slow in CI. The
GITHUB_ACTIONScheck appropriately skips this test in CI environments while allowing local validation.clients/typescript/test/framework.ts (1)
29-29: LGTM! Bundle support added to test framework.The additions of
ExampleBundleprogram, related PDAs (bundlePositionEntity1Pda,bundleVelocityEntity1Pda), and initialization logic are consistent with the PR's bundling feature and follow the existing patterns in the test framework.Also applies to: 37-37, 64-66, 74-74
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.cs (2)
59-95: LGTM! Bundle-aware overload correctly implemented.The new overload correctly uses
component.Seeds(seed)for PDA derivation andcomponent.Programas the program ID, aligning with bundle-specific component handling. The documentation clearly explains the bundle-specific behavior.
22-57: Verify Rust client discriminator consistency
The 8-byte discriminator{90, 147, 75, 178, 85, 88, 4, 137}is present in TypeScript and C# implementations but not found in Rust; ensure the Rust client defines and uses the same value.clients/csharp/Solana.Unity.Bolt/WorldProgram/Generated.cs (1)
435-465: LGTM! Discriminator serialization correctly implemented.The discriminator serialization follows a consistent pattern across all updated methods:
- Single discriminators: length (S32) + data
- Array discriminators: count (S32) + per-element (length + data)
These changes align with the PR's discriminator-based routing feature. As this is generated code, ensure the IDL and code generator are kept in sync for future updates.
Also applies to: 467-497, 513-529, 531-547
crates/bolt-lang/Cargo.toml (1)
19-19: LGTM! Dependency update aligns with bundle feature.The replacement of
bolt-attribute-bolt-programwithbolt-attribute-bolt-bundlecorrectly supports the new bundling feature introduced in this PR.crates/bolt-lang/src/errors.rs (1)
11-16: LGTM! Error variants correctly added.The new error variants
AccountMismatchandComponentNotDelegateableare well-documented and support the PR's bundle and delegation features. The error messages are clear and actionable.crates/bolt-lang/attribute/system/Cargo.toml (1)
17-18: LGTM! Dependencies correctly added.The restoration of
proc-macro2and addition ofbolt-attributeas workspace dependencies align with the PR's attribute macro reorganization and maintain consistency across the workspace.crates/bolt-lang/attribute/bundle/Cargo.toml (1)
14-15: LGTM! Dependency consolidation improves maintainability.Replacing individual workspace dependencies with
bolt-attribute.workspace = truecentralizes version management and reduces configuration duplication.clients/typescript/src/ecs/index.ts (1)
1-2: Barrel exports validated.component.tsandsystem.tsare present; exports are correct.crates/bolt-lang/attribute/system/src/lib.rs (1)
4-5: LGTM—delegation validated
bolt_attribute::system::processexists with the expected signature; parameter rename is correct.clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt.cs (1)
30-40: Confirm discriminator implementations match across languages. The C#, Rust, and TypeScript implementations all hash UTF-8 bytes and truncate to the first 8 bytes of the SHA-256 digest, yielding identical discriminators.crates/bolt-cli/src/templates/mod.rs (1)
1-1: Bundle module existsThe file
crates/bolt-cli/src/templates/bundle/mod.rsis present; module declaration is correct.crates/bolt-lang/attribute/src/lib.rs (1)
1-7: LGTM!The module structure is clean and follows standard Rust conventions for organizing procedural macro crates. The public modules expose the bundle, component, delegate, and system attribute macros, while keeping common utilities private.
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (2)
12-12: LGTM!The new
using Bolt;import enables access to the updated World API surface.
51-60: LGTM!The refactored code uses the new
Bolt.World.ApplySystemAPI, which is cleaner and more intuitive than manually constructing account metadata. The entity-type-based approach with explicit component associations improves clarity.crates/bolt-cli/src/lib.rs (5)
4-4: LGTM!The bundle module is properly added to the CLI structure.
14-14: LGTM!The import correctly brings the
new_bundlefunction into scope for command routing.
44-45: LGTM!The Bundle command variant is correctly added with appropriate help text.
76-79: LGTM!The
BundleCommandstruct follows the same pattern asComponentCommandandSystemCommand, maintaining consistency in the CLI design.
198-198: LGTM!The bundle command is correctly routed to the
new_bundlefunction with the appropriate parameters.clients/typescript/test/low-level/ecs.ts (1)
103-103: LGTM!The discriminator usage is consistent across all component initialization and destruction operations, correctly passing
GetDiscriminator("global:initialize")andGetDiscriminator("global:destroy")as required by the new discriminator-based routing mechanism.Also applies to: 125-125, 146-146, 167-167, 449-449
clients/typescript/src/ecs/system.ts (1)
1-12: LGTM!The
Systemclass implementation is clean and follows the same pattern as theComponentclass. The staticfromfactory method provides convenient type conversion for accepting either aPublicKeyorSysteminstance.clients/typescript/src/index.ts (4)
5-5: LGTM!The
cryptoimport is correctly added to support theGetDiscriminatorfunction.
20-20: LGTM!Exporting
ComponentandSystemmakes these domain objects available for use in world transactions and tests.
26-28: LGTM!The
GetDiscriminatorfunction correctly generates an 8-byte discriminator using SHA-256, matching Anchor's discriminator generation pattern. This enables discriminator-based instruction routing for bundled components and systems.
118-118: LGTM!The seed buffer creation and PDA derivation logic correctly handles optional seeds.
Also applies to: 121-122
crates/bolt-cli/src/bundle.rs (1)
7-42: LGTM!The
new_bundlefunction correctly follows the established pattern for creating new programs (components/systems). It properly validates uniqueness, updates the configuration, and writes the changes toAnchor.toml.clients/csharp/Solana.Unity.Bolt/ECS/System.cs (1)
1-27: LGTM!The
Systemclass implementation is clean and well-documented. It correctly mirrors the TypeScriptSystem.frompattern in C# with proper type checking and error handling. The factory method provides flexible type conversion for accepting eitherPublicKeyorSysteminstances.crates/bolt-lang/src/lib.rs (3)
10-12: LGTM: module surface extended.
16-16: LGTM: re-exportingbundleis consistent with the new API.
72-75: LGTM:NumberOfComponentstrait is clear and minimal.clients/csharp/Solana.Unity.Bolt.Test/ECSTest.cs (5)
30-35: LGTM: added bundled component init steps to the test flow.
54-59: LGTM: added bundled system apply steps to the test flow.
94-109: LGTM: bundled Position init + structured deserialization checks.
158-168: LGTM: switched toSerializeArgsoverload for explicit serialization.
199-220: LGTM: bundled Movement apply with domain objects and component PDAs.crates/bolt-lang/src/instructions/update.rs (1)
6-31: Data write path looks correct.Authority check, CPI check, and discriminator offset write are sound.
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/InitializeComponent.cs (1)
31-41: Approve CPI auth wiring and account ordering InitializeComponentAccounts defines CpiAuth, and WorldProgram.InitializeComponent uses the same account order expected by the on-chain handler.clients/csharp/Solana.Unity.Bolt/WorldProgram/World.cs (1)
231-247: Approve code changes. CpiAuth and Authority are correctly keyed; please manually verify that the on-chain Apply handlers expect CpiAuth at this index.crates/bolt-lang/attribute/src/component/generate/program.rs (3)
59-70: LGTM!The transformation correctly enables Apply structs to accept any compatible program by changing Program fields to UncheckedAccount with appropriate CHECK attributes.
80-132: LGTM!The destroy function correctly handles both bundled and standalone components by conditionally adapting PDA seeds. Bundled components use the component name as a seed prefix, while standalone components use the component type's seed method.
135-187: LGTM!The initialize function correctly handles both bundled and standalone components with consistent PDA seed derivation. The use of
init_if_neededis appropriate for component initialization flows.crates/bolt-lang/attribute/src/system/mod.rs (5)
35-83: LGTM!The process function correctly handles standalone system processing by:
- Extracting component bundle information via visitor
- Injecting necessary imports and structs
- Generating the bolt_execute wrapper
- Applying AST transformations
The panic with a descriptive message at line 78-81 is appropriate for proc macro error handling.
85-194: LGTM!The bundling transform correctly handles:
- Renaming components/bumps structs with suffix to prevent collisions
- Renaming execute functions uniquely per system
- Generating appropriately named bolt_execute wrappers
- Supporting both bundled (with suffix) and standalone (without suffix) paths
196-262: LGTM!The visitor correctly transforms system execute functions by:
- Injecting required lifetimes
- Converting return types to
Result<Vec<Vec<u8>>>- Wrapping Ok expressions with try_to_vec() for serialization
- Handling nested control flow (loops, if statements, blocks)
382-417: LGTM!The return type checking correctly handles both
Result<Vec<Vec<u8>>>andResult<(Vec<u8>, ...)>patterns, ensuring proper type validation for system execute functions.
448-484: LGTM!The argument modification correctly transforms custom types to
Vec<u8>and inserts appropriate deserialization logic withparse_args. This enables systems to accept serialized arguments from the world program.crates/bolt-lang/attribute/src/delegate/mod.rs (1)
86-192: LGTM!The delegate account structures and remaining functions (process_undelegation, undelegate) are correctly implemented with:
- Appropriate account annotations (mut, CHECK comments)
- Proper error handling with Result types
- Correct calls to bolt_lang delegation helpers
clients/typescript/src/world/transactions.ts (3)
400-446: InitializeComponent method name verified
The Rust implementation containspub fn initializeincrates/bolt-lang/src/instructions/initialize.rs, so the TypeScript binding aligns correctly.
345-388: No action required:destroymethod matches Rust implementation
pub fn destroy<'info>exists incrates/bolt-lang/src/instructions/destroy.rs.
457-569: Approve bundling logic usingsystem.name. Verified thatSystem.frompreservesnameand thatgetMethodDiscriminatorleverages it for discriminators.
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overral LGTM, but
- We should remove all hashing, conversion from string to discriminator should be off-chain
- Several places hash a const string, this should be replaced with a const
- Please address linter and code rabbit comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (6)
crates/programs/world/src/lib.rs (6)
264-306: Critical: Propagate authority signer flag in CPI.Line 280 hardcodes
is_signer = falsefor the authority account meta. When the caller signs (the common case), the downstream component program will observe the account as non-signer and fail withMissingRequiredSignature.Apply this diff to propagate the actual signer bit:
let accounts = vec![ AccountMeta::new_readonly(ctx.accounts.cpi_auth.key(), true), AccountMeta::new(ctx.accounts.payer.key(), true), AccountMeta::new(ctx.accounts.data.key(), false), AccountMeta::new_readonly(ctx.accounts.entity.key(), false), - AccountMeta::new_readonly(ctx.accounts.authority.key(), false), + AccountMeta::new_readonly( + ctx.accounts.authority.key(), + ctx.accounts.authority.is_signer, + ), AccountMeta::new_readonly(ctx.accounts.system_program.key(), false), ];
348-365: Major: Move discriminator computation to compile-time constant.Line 354 computes the discriminator at runtime on every
applycall. Sincediscriminator_foris aconst fn, this should be computed once at compile time to avoid wasting compute units.Based on past review comments.
Apply this diff to declare the discriminator as a constant:
+const BOLT_EXECUTE_DISCRIMINATOR: [u8; 8] = discriminator_for("global:bolt_execute"); + pub fn apply<'info>( ctx: Context<'_, '_, '_, 'info, Apply<'info>>, args: Vec<u8>, ) -> Result<()> { apply_impl( ctx, - discriminator_for("global:bolt_execute").to_vec(), + BOLT_EXECUTE_DISCRIMINATOR.to_vec(), args, ) }
382-399: Major: Use compile-time constant for discriminator.Line 388 has the same runtime computation issue. Use the constant suggested for the
applyfunction above.Based on past review comments.
Apply this diff:
pub fn apply_with_session<'info>( ctx: Context<'_, '_, '_, 'info, ApplyWithSession<'info>>, args: Vec<u8>, ) -> Result<()> { apply_with_session_impl( ctx, - discriminator_for("global:bolt_execute").to_vec(), + BOLT_EXECUTE_DISCRIMINATOR.to_vec(), args, ) }
420-466: Major: Declare update discriminator as compile-time constant.Line 444 computes the
global:updatediscriminator at runtime on every component update CPI.Based on past review comments.
Apply this diff:
+const UPDATE_DISCRIMINATOR: [u8; 8] = discriminator_for("global:update"); + pub fn apply_impl<'info>( // ... parameters ) -> Result<()> { // ... existing code for ((program, component), result) in pairs.into_iter().zip(results.into_iter()) { let accounts = vec![ AccountMeta::new_readonly(ctx.accounts.cpi_auth.key(), true), AccountMeta::new(component.key(), false), AccountMeta::new_readonly(ctx.accounts.authority.key(), true), ]; - let mut data = discriminator_for("global:update").to_vec(); + let mut data = UPDATE_DISCRIMINATOR.to_vec(); // ... rest of function
468-516: Major: Declare session update discriminator as compile-time constant.Line 493 computes the
global:update_with_sessiondiscriminator at runtime.Based on past review comments.
Apply this diff:
+const UPDATE_WITH_SESSION_DISCRIMINATOR: [u8; 8] = discriminator_for("global:update_with_session"); + pub fn apply_with_session_impl<'info>( // ... parameters ) -> Result<()> { // ... existing code for ((program, component), result) in pairs.into_iter().zip(results.into_iter()) { let accounts = vec![ AccountMeta::new_readonly(ctx.accounts.cpi_auth.key(), true), AccountMeta::new(component.key(), false), AccountMeta::new_readonly(ctx.accounts.authority.key(), true), AccountMeta::new_readonly(ctx.accounts.session_token.key(), false), ]; - let mut data = discriminator_for("global:update_with_session").to_vec(); + let mut data = UPDATE_WITH_SESSION_DISCRIMINATOR.to_vec(); // ... rest of function
565-577: Critical: Preserve writable and signer flags when routing to systems.Lines 566-570 rewrite every remaining account as
AccountMeta::new_readonly(..., false), dropping both writable and signer privileges. Systems that need to mutate components will trap withInstructionError::ReadonlyAccount.Apply this diff to mirror the original flags:
let mut accounts = vec![AccountMeta::new_readonly(authority.key(), false)]; - accounts.extend( - remaining_accounts - .iter() - .map(|account| AccountMeta::new_readonly(account.key(), false)), - ); + accounts.extend(remaining_accounts.iter().map(|account| { + if account.is_writable { + AccountMeta::new(account.key(), account.is_signer) + } else { + AccountMeta::new_readonly(account.key(), account.is_signer) + } + }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
crates/programs/world/src/lib.rs(6 hunks)crates/programs/world/src/utils.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/programs/world/src/lib.rs (5)
crates/programs/world/src/utils.rs (1)
discriminator_for(1-17)clients/typescript/src/world/transactions.ts (2)
InitializeComponent(400-446)DestroyComponent(345-388)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/InitializeComponent.cs (1)
World(10-72)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DestroyComponent.cs (1)
World(10-68)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt.cs (1)
World(13-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint (yarn)
- GitHub Check: lint (clippy)
- GitHub Check: setup
🔇 Additional comments (3)
crates/programs/world/src/lib.rs (2)
6-7: LGTM!The utils module import and
discriminator_forfunction are correctly added to support discriminator-based routing.
308-346: LGTM!The manual CPI construction is correct. The authority is correctly marked as a signer since the
DestroyComponentaccounts struct declares it asSigner<'info>.crates/programs/world/src/utils.rs (1)
1-17: Discriminator format consistent across codebase. The 8-byte SHA-256 truncation inutils.rsaligns with macro-generated discriminators (bolt-lang), TypeScript and C# client implementations, and CPI usages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (2)
crates/programs/world/src/lib.rs (2)
280-280: Critical: Restore authority signer flag in CPI call.This is the same critical issue flagged in the previous review. The authority's
is_signerflag is hardcoded tofalse, but when the caller actually signs (the common case), the downstream component program will observe the account as non-signer and fail withMissingRequiredSignature.Apply this diff to propagate the real signer bit:
let accounts = vec![ AccountMeta::new_readonly(ctx.accounts.cpi_auth.key(), true), AccountMeta::new(ctx.accounts.payer.key(), true), AccountMeta::new(ctx.accounts.data.key(), false), AccountMeta::new_readonly(ctx.accounts.entity.key(), false), - AccountMeta::new_readonly(ctx.accounts.authority.key(), false), + AccountMeta::new_readonly( + ctx.accounts.authority.key(), + ctx.accounts.authority.is_signer, + ), AccountMeta::new_readonly(ctx.accounts.system_program.key(), false), ];
561-566: Critical: Preserve writable/signer flags when routing to systems.This is the same critical issue flagged in the previous review. The code maps every remaining account to
AccountMeta::new_readonly(..., false), which:
- Drops the signer flag from the authority account (line 561)
- Converts all writable accounts to read-only (lines 562-566)
- Drops all signer privileges from remaining accounts
Any system that needs to mutate a component will trap with
InstructionError::ReadonlyAccount.Apply this diff to preserve flags:
- let mut accounts = vec![AccountMeta::new_readonly(authority.key(), false)]; - accounts.extend( - remaining_accounts - .iter() - .map(|account| AccountMeta::new_readonly(account.key(), false)), - ); + let mut accounts = vec![AccountMeta::new_readonly(authority.key(), authority.is_signer)]; + accounts.extend(remaining_accounts.iter().map(|account| { + if account.is_writable { + AccountMeta::new(account.key(), account.is_signer) + } else { + AccountMeta::new_readonly(account.key(), account.is_signer) + } + }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
crates/programs/world/src/lib.rs(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/programs/world/src/lib.rs (4)
crates/programs/world/src/utils.rs (1)
discriminator_for(1-17)clients/typescript/src/world/transactions.ts (2)
InitializeComponent(400-446)DestroyComponent(345-388)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DestroyComponent.cs (1)
World(10-68)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/InitializeComponent.cs (1)
World(10-72)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: setup
- GitHub Check: setup
🔇 Additional comments (4)
crates/programs/world/src/lib.rs (4)
6-7: LGTM: Utility module integration.The addition of the
utilsmodule withdiscriminator_forfunction supports the discriminator-based routing architecture introduced in this PR.
308-346: LGTM: Discriminator-based destroy CPI.The manual CPI construction for
destroy_componentcorrectly propagates account metadata and uses the discriminator parameter for bundle routing.
348-362: LGTM: Const discriminator addresses efficiency concerns.The use of
const DISCRIMINATORat compile-time (line 352) addresses the efficiency concerns raised in past reviews about string-to-discriminator conversion overhead. The newapply_with_discriminatorvariant provides flexibility for bundle routing.
553-553: Avoid unnecessary assignment.The
system_discriminatoris already aVec<u8>, so directly using it as the initial value fordatais more efficient than reassigning.Apply this diff:
- let mut data = system_discriminator; + let mut data = system_discriminator.clone(); let len_le = (args.len() as u32).to_le_bytes(); data.extend_from_slice(&len_le); data.extend_from_slice(args.as_slice());Note: Actually, since
system_discriminatoris moved here, keep the current approach but be aware that the parameter is consumed.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
crates/programs/world/src/lib.rs (4)
284-284: CRITICAL: Authority signer flag still hardcoded - previous review issue unresolved.This critical issue from the previous review remains unfixed. Line 284 hardcodes
is_signer = falsefor the authority account, which will causeMissingRequiredSignatureerrors when the authority actually signs the transaction (the common case).Apply this diff to propagate the real signer flag:
let accounts = vec![ AccountMeta::new_readonly(ctx.accounts.cpi_auth.key(), true), AccountMeta::new(ctx.accounts.payer.key(), true), AccountMeta::new(ctx.accounts.data.key(), false), AccountMeta::new_readonly(ctx.accounts.entity.key(), false), - AccountMeta::new_readonly(ctx.accounts.authority.key(), false), + AccountMeta::new_readonly( + ctx.accounts.authority.key(), + ctx.accounts.authority.is_signer, + ), AccountMeta::new_readonly(ctx.accounts.system_program.key(), false), ];
268-271: Optimize discriminator parameter type and eliminate unnecessary allocation.The function accepts
discriminator: Vec<u8>which requires heap allocation, and Line 288 calls.to_vec()creating an unnecessary clone of the already-owned Vec.Apply this diff to optimize:
pub fn initialize_component( ctx: Context<InitializeComponent>, - discriminator: Vec<u8>, + discriminator: [u8; 8], ) -> Result<()> { if !ctx.accounts.authority.is_signer && ctx.accounts.authority.key != &ID { return Err(WorldError::InvalidAuthority.into()); } // Pure Solana SDK logic for CPI to bolt_component::initialize use anchor_lang::solana_program::instruction::{AccountMeta, Instruction}; // Prepare the accounts for the CPI let accounts = vec![ AccountMeta::new_readonly(ctx.accounts.cpi_auth.key(), true), AccountMeta::new(ctx.accounts.payer.key(), true), AccountMeta::new(ctx.accounts.data.key(), false), AccountMeta::new_readonly(ctx.accounts.entity.key(), false), AccountMeta::new_readonly(ctx.accounts.authority.key(), false), AccountMeta::new_readonly(ctx.accounts.system_program.key(), false), ]; - let data = discriminator.to_vec(); + let data = discriminator.to_vec(); let ix = Instruction { program_id: ctx.accounts.component_program.key(), accounts, data, };Since discriminators are fixed-size (8 bytes based on SHA-256), use
[u8; 8]for compile-time size checking and to avoid heap allocation.Also applies to: 288-288
312-312: Optimize discriminator parameter and eliminate unnecessary allocation.Similar to
initialize_component, the discriminator parameter should be[u8; 8]and Line 327's.to_vec()call is unnecessary.Apply this diff:
-pub fn destroy_component(ctx: Context<DestroyComponent>, discriminator: Vec<u8>) -> Result<()> { +pub fn destroy_component(ctx: Context<DestroyComponent>, discriminator: [u8; 8]) -> Result<()> { // Pure Solana SDK logic for CPI to bolt_component::destroy use anchor_lang::solana_program::instruction::{AccountMeta, Instruction}; // Prepare the accounts for the CPI (must match bolt_component::Destroy) let accounts = vec![ AccountMeta::new_readonly(ctx.accounts.cpi_auth.key(), true), AccountMeta::new_readonly(ctx.accounts.authority.key(), true), AccountMeta::new(ctx.accounts.receiver.key(), false), AccountMeta::new_readonly(ctx.accounts.entity.key(), false), AccountMeta::new(ctx.accounts.component.key(), false), AccountMeta::new_readonly(ctx.accounts.component_program_data.key(), false), AccountMeta::new_readonly(ctx.accounts.system_program.key(), false), ]; - let data = discriminator.to_vec(); + let data = discriminator.to_vec(); let ix = Instruction { program_id: ctx.accounts.component_program.key(), accounts, data, };Also applies to: 327-327
562-566: CRITICAL: Account metadata flags still being dropped - previous review issue unresolved.This critical issue from the previous review remains unfixed. Lines 562-566 map all remaining accounts to
AccountMeta::new_readonly(..., false), which drops bothis_writableandis_signerflags. Any system that needs to mutate a component will trap withInstructionError::ReadonlyAccount.Apply this diff to preserve the original flags:
let mut accounts = vec![AccountMeta::new_readonly(authority.key(), false)]; - accounts.extend( - remaining_accounts - .iter() - .map(|account| AccountMeta::new_readonly(account.key(), false)), - ); + accounts.extend(remaining_accounts.iter().map(|account| { + if account.is_writable { + AccountMeta::new(account.key(), account.is_signer) + } else { + AccountMeta::new_readonly(account.key(), account.is_signer) + } + }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
crates/programs/world/src/lib.rs(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/programs/world/src/lib.rs (5)
crates/programs/world/src/utils.rs (1)
discriminator_for(1-17)clients/typescript/src/world/transactions.ts (2)
InitializeComponent(400-446)DestroyComponent(345-388)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DestroyComponent.cs (1)
World(10-68)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/InitializeComponent.cs (1)
World(10-72)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt.cs (1)
World(13-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: setup
- GitHub Check: setup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 15
♻️ Duplicate comments (6)
crates/bolt-lang/attribute/src/common/mod.rs (1)
16-31: Remove unnecessary clone.The
.clone()at line 29 is wasteful since:
- The function mutates
itemsin place viaitems.insert().- The return value is never used by the caller at line 12.
- Cloning a
Vec<syn::Item>can be expensive for large modules.Apply this diff to remove the unnecessary clone:
- (brace, items.clone()) + (brace, items)Alternatively, since the return value is unused, consider simplifying the function signature to return
()instead of theOptiontuple.crates/programs/world/src/lib.rs (1)
268-310: Restore authority signer flag in CPI call.The
initialize_componentfunction hardcodesis_signer = falsefor the authority account meta at line 284. When the caller actually signs (the common case), the downstream component program will observe the account as non-signer and fail withMissingRequiredSignature.Apply this diff to propagate the real signer bit:
let accounts = vec![ AccountMeta::new_readonly(ctx.accounts.cpi_auth.key(), true), AccountMeta::new(ctx.accounts.payer.key(), true), AccountMeta::new(ctx.accounts.data.key(), false), AccountMeta::new_readonly(ctx.accounts.entity.key(), false), - AccountMeta::new_readonly(ctx.accounts.authority.key(), false), + AccountMeta::new_readonly( + ctx.accounts.authority.key(), + ctx.accounts.authority.is_signer, + ), AccountMeta::new_readonly(ctx.accounts.system_program.key(), false), ];crates/bolt-lang/attribute/src/system/mod.rs (3)
295-303: Handle parse errors defensively.The
unwrap()at line 302 could panic with an unclear error message if the generated code fails to parse. While the quoted code should always be valid, defensive error handling would improve debuggability.Based on previous review feedback. Apply this diff:
- content.push(syn::parse2(initialize_extra_accounts).unwrap()); + content.push( + syn::parse2(initialize_extra_accounts) + .expect("Failed to parse generated init_extra_accounts function") + );
337-368: Use defensive access pattern for path segments.The
unwrap()at line 340 could panic iftp.path.segmentsis empty, even though the code assumes it has at least one element from the earlier match.Based on previous review feedback. Apply this diff:
if let Some(components_ty) = components_ty_opt { let components_with_info: Type = match components_ty { Type::Path(mut tp) => { - let seg = tp.path.segments.last_mut().unwrap(); + let seg = tp.path.segments.last_mut() + .expect("Component type path has no segments"); match &mut seg.arguments {
486-511: Consider safer access pattern.The
expect()at line 494 could panic iftype_path.path.segmentsis empty. While the earlier checks should ensure this doesn't happen, consider using a more defensive pattern.Based on previous review feedback (though it mentioned
unwrap(), the code actually usesexpect()). The current implementation is acceptable, but for consistency you could use:let last_segment = type_path .path .segments .last() - .expect("Context segment not found"); + .ok_or_else(|| { + panic!("Context type path has no segments in system execution signature") + }) + .unwrap();However, given the context and earlier type checks, the current
expect()is sufficient.crates/bolt-lang/attribute/src/delegate/mod.rs (1)
49-51: Critical: PDA seed mismatch between delegate and component initialization remains unfixed.This issue was previously flagged but not resolved. The delegate function constructs PDA seeds including both
#component::seed()and#name.as_bytes()(line 50), but bundled components should only use[#name.as_bytes(), &ctx.accounts.entity.key().to_bytes()].Reference:
crates/bolt-lang/attribute/src/component/generate/program.rsline 100 shows bundled components use[name_bytes, entity.key().as_ref()]without the component seed prefix.Apply this diff to fix the seed construction for bundled components:
let component_matches = components.iter().map(|(component, name)| quote! { - #component::DISCRIMINATOR => &[#component::seed(), #name.as_bytes(), &ctx.accounts.entity.key().to_bytes()] + #component::DISCRIMINATOR => &[#name.as_bytes(), &ctx.accounts.entity.key().to_bytes()] }).collect::<Vec<_>>();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.cs(1 hunks)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/InitializeComponent.cs(2 hunks)clients/typescript/src/ecs/component.ts(1 hunks)crates/bolt-lang/attribute/component-deserialize/src/lib.rs(3 hunks)crates/bolt-lang/attribute/src/common/mod.rs(1 hunks)crates/bolt-lang/attribute/src/component/generate/program.rs(1 hunks)crates/bolt-lang/attribute/src/delegate/mod.rs(1 hunks)crates/bolt-lang/attribute/src/system/mod.rs(1 hunks)crates/bolt-lang/attribute/system-input/src/lib.rs(2 hunks)crates/bolt-lang/src/lib.rs(3 hunks)crates/programs/world/src/lib.rs(6 hunks)crates/programs/world/src/utils.rs(1 hunks)docs/REPORT.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-15T15:51:36.717Z
Learnt from: notdanilo
PR: magicblock-labs/bolt#207
File: crates/programs/world/src/lib.rs:519-519
Timestamp: 2025-10-15T15:51:36.717Z
Learning: In the Bolt codebase, discriminator parameters should remain as `Vec<u8>` rather than `[u8; 8]` because the client code generation tooling (TypeScript/C#) cannot generate code from fixed-size array types.
Applied to files:
crates/programs/world/src/utils.rscrates/bolt-lang/attribute/component-deserialize/src/lib.rscrates/programs/world/src/lib.rs
🧬 Code graph analysis (8)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/InitializeComponent.cs (5)
clients/csharp/Solana.Unity.Bolt/WorldProgram/World.cs (15)
PublicKey(36-46)PublicKey(48-55)PublicKey(57-66)PublicKey(68-77)PublicKey(79-91)PublicKey(93-96)PublicKey(98-105)PublicKey(106-111)PublicKey(113-119)PublicKey(121-132)PublicKey(136-143)PublicKey(145-152)PublicKey(188-191)PublicKey(192-200)WorldProgram(17-357)clients/csharp/Solana.Unity.Bolt/WorldProgram/Generated.cs (4)
WorldProgram(387-606)InitializeComponentAccounts(328-343)Entity(27-49)Entity(34-48)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt.cs (1)
GetDiscriminator(30-40)clients/csharp/Solana.Unity.Bolt/ECS/Component.cs (4)
Component(9-34)Component(15-15)Component(20-25)Seeds(30-33)clients/csharp/Solana.Unity.Bolt/ECS/Identifier.cs (1)
GetMethodDiscriminator(33-37)
clients/typescript/src/ecs/component.ts (2)
clients/typescript/src/index.ts (2)
Component(20-20)FindComponentPda(109-124)clients/typescript/src/ecs/identifier.ts (1)
Identifier(4-18)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.cs (3)
clients/csharp/Solana.Unity.Bolt/WorldProgram/World.cs (15)
PublicKey(36-46)PublicKey(48-55)PublicKey(57-66)PublicKey(68-77)PublicKey(79-91)PublicKey(93-96)PublicKey(98-105)PublicKey(106-111)PublicKey(113-119)PublicKey(121-132)PublicKey(136-143)PublicKey(145-152)PublicKey(188-191)PublicKey(192-200)WorldProgram(17-357)clients/typescript/src/index.ts (1)
FindComponentPda(109-124)clients/csharp/Solana.Unity.Bolt/ECS/Component.cs (4)
Component(9-34)Component(15-15)Component(20-25)Seeds(30-33)
crates/bolt-lang/attribute/src/delegate/mod.rs (3)
crates/programs/world/src/lib.rs (6)
seed(740-742)seed(799-801)seed(827-829)seed(837-839)pda(748-750)pda(807-809)crates/bolt-lang/src/lib.rs (2)
seed(61-61)discriminator_from_account_info(96-101)crates/bolt-lang/attribute/src/component/generate/program.rs (2)
vec(36-39)vec(235-243)
crates/bolt-lang/src/lib.rs (1)
crates/bolt-lang/attribute/bundle/src/lib.rs (1)
bundle(9-11)
crates/bolt-lang/attribute/src/component/generate/program.rs (4)
crates/bolt-lang/src/lib.rs (2)
seed(61-61)size(62-62)crates/bolt-lang/src/instructions/destroy.rs (1)
destroy(5-39)crates/bolt-lang/src/instructions/initialize.rs (1)
initialize(4-14)crates/bolt-lang/src/instructions/update.rs (2)
update(6-31)update_with_session(33-78)
crates/bolt-lang/attribute/src/system/mod.rs (2)
crates/bolt-lang/attribute/src/bundle/mod.rs (1)
process(10-80)crates/bolt-lang/src/lib.rs (1)
parse_args(50-55)
crates/programs/world/src/lib.rs (2)
crates/programs/world/src/utils.rs (1)
discriminator_for(19-36)clients/typescript/src/world/transactions.ts (2)
InitializeComponent(400-446)DestroyComponent(345-388)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: setup
- GitHub Check: setup
🔇 Additional comments (35)
crates/bolt-lang/attribute/src/common/mod.rs (2)
5-14: LGTM!The function correctly converts the identifier to snake_case and generates a module with the appropriate structure.
23-26: Past issue resolved.The
authorityfield is correctly typed asSigner<'info>, which enforces signature verification at the type level. The previous concern about usingAccountInfo<'info>has been addressed.clients/typescript/src/ecs/component.ts (4)
6-8: LGTM!Constructor correctly delegates to the parent
Identifierclass with bothprogramand optionalnameparameters.
10-14: LGTM!The static factory method provides a clean type coercion pattern. When converting from
PublicKey, the resultingComponentwill havename = undefined, which correctly represents a standalone (non-bundled) component.
16-22: LGTM!PDA derivation correctly uses
this.programas the component ID and passes the concatenated seeds from theseeds()method toFindComponentPda.
24-26: Previous critical issue has been resolved.The seed concatenation order is now correct: component name (
this.name) prefixes the caller-provided seed, which aligns with the bundle specification. This fix resolves the critical PDA derivation issue flagged in the previous review.crates/programs/world/src/lib.rs (2)
6-14: LGTM: Efficient compile-time discriminators.Using
const fn discriminator_forto compute discriminators at compile time is an excellent design choice that avoids runtime hashing overhead.
561-574: Excellent fix: Account metadata flags now preserved.The manual
AccountMetaconstruction at lines 568-574 correctly preserves bothis_signerandis_writableflags from the originalAccountInfoinstances. This addresses the critical issue from the previous review where all remaining accounts were being hardcoded as read-only with no signer privileges.crates/programs/world/src/utils.rs (1)
1-36: LGTM: Well-documented discriminator function.The comprehensive documentation covers the function's purpose, collision risk considerations, and includes an example. The manual loop at line 28-33 is appropriately commented to explain the const fn compatibility requirement. All concerns from the previous review have been addressed.
crates/bolt-lang/attribute/system-input/src/lib.rs (2)
79-79: LGTM: Correct type strengthening for authority.Changing the authority field from
AccountInfo<'info>toSigner<'info>is the correct security posture. This ensures the authority is verified as a signer at the Anchor framework level.
100-106: LGTM: Cleaner trait architecture.Removing the local
NumberOfComponentstrait definition and implementingbolt_lang::NumberOfComponentsinstead centralizes this concept in the core library, reducing duplication and improving maintainability.crates/bolt-lang/src/lib.rs (3)
10-11: LGTM: Module additions for discriminator infrastructure.The new
cpiandinstructionsmodules provide the necessary infrastructure for discriminator-based CPI operations in the bundle architecture.
72-75: LGTM: Centralized trait for component counting.The
NumberOfComponentstrait provides a clean abstraction for system inputs to declare their component count, used by the system-input attribute processor.
88-102: Excellent fix: Safe deserialization with proper bounds checking.The
BoltMetadatamethods now include:
- Length checks before indexing (
require!(data.len() >= ...))- Use of
BoltMetadata::INIT_SPACEinstead ofmem::size_offor correct Borsh serialized size- Proper error handling with
ErrorCode::AccountDidNotDeserializeThis addresses all concerns from the previous review and prevents panics on malformed account data.
crates/bolt-lang/attribute/src/system/mod.rs (5)
1-17: LGTM!The imports and struct definitions are appropriate for procedural macro processing using the visitor pattern.
19-33: LGTM!The wrapper generation logic correctly adapts the execute function signature and handles component conversion with proper error handling.
381-416: LGTM!The type checking logic correctly identifies
Result<Vec<Vec<u8>>>andResult<Vec<u8>>return types with proper defensive pattern matching.
418-445: LGTM!The expression extraction logic correctly identifies and extracts inner
Ok(...)expressions with proper defensive coding.
447-483: LGTM!The argument transformation logic correctly converts typed arguments to
Vec<u8>and injects the necessary deserialization code.crates/bolt-lang/attribute/src/delegate/mod.rs (4)
1-4: LGTM!Standard imports for procedural macro code generation are in place.
6-36: LGTM!The function correctly uses
.expect()with a clear error message (line 31), addressing the previous review feedback about replacing.unwrap()calls.
53-84: LGTM with dependency on seed fix.The delegate function logic correctly extracts the discriminator, matches components, and calls
delegate_accountwith proper account construction. Once the PDA seed construction issue (lines 49-51) is resolved, this function will work correctly.
184-192: LGTM!The function correctly returns all six generated items as a tuple.
crates/bolt-lang/attribute/component-deserialize/src/lib.rs (2)
42-44: LGTM! Discriminator computation now follows Anchor's pattern.The SHA-256-based discriminator computation correctly uses the
"account:{name}"format, which aligns with Anchor's account discriminator pattern. The previous review concern about the missing"account:"prefix has been addressed.
77-77: LGTM! DISCRIMINATOR constant implementation is correct.The discriminator is properly expanded from the computed
Vec<u8>into a static byte array, following the standard AnchorDiscriminatortrait pattern.crates/bolt-lang/attribute/src/component/generate/program.rs (6)
1-11: LGTM!The imports and
remove_component_attributesfunction are well-structured and appropriate for proc macro code generation.
13-23: LGTM!The
generate_instructionsfunction correctly constructs a type path and delegates to the module modifier.
58-70: LGTM once the caller is fixed.The
modify_apply_structfunction correctly mutates the struct fields. It will work properly once the critical issue inmodify_component_module(lines 34-55) is resolved.
72-77: LGTM!The CHECK attribute generation is appropriate for documenting unchecked accounts.
134-187: LGTM with the same refactoring suggestion.The
generate_initializefunction logic is correct. The same PDA seed generation duplication mentioned forgenerate_destroyapplies here (lines 153-158).
191-232: LGTM!The generated update functions and account structs are well-structured and correctly call the underlying
bolt_lang::instructionsfunctions.clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.cs (1)
97-109: LGTM!The
Concathelper correctly concatenates byte arrays usingBuffer.BlockCopy. The implementation is straightforward and efficient.clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/InitializeComponent.cs (3)
6-6: LGTM!The
using System;directive is correctly added to support the newArgumentNullExceptionandArgumentExceptionused in the validation logic.
38-41: LGTM!The addition of
CpiAuth = WorldProgram.CpiAuthAddressand the discriminator change toGetDiscriminator("global:initialize")correctly align with the discriminator-based routing introduced in this PR for supporting both standalone and bundled components.
57-75: LGTM! Past review feedback addressed.The new bundled component overload correctly implements the validation requested in the previous review:
- Validates component is not null (line 58)
- Validates discriminator length is 8 bytes (line 60)
- Uses component-specific discriminator via
GetMethodDiscriminator("initialize")- Uses
component.Seeds(seed)for proper PDA derivation in bundlesThe implementation properly mirrors the TypeScript client behavior for bundled components.
|
|
||
| // Build instruction data per TS beet struct: | ||
| // discriminator[8] + commitFrequencyMs[u32 le] + validator[COption<Pubkey>] + pdaSeeds[Vec<Bytes>] | ||
| byte[] discriminator = new byte[] { 90, 147, 75, 178, 85, 88, 4, 137 }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Hardcoded discriminator may break bundled components.
The discriminator { 90, 147, 75, 178, 85, 88, 4, 137 } is hardcoded for all components. For bundled components, the discriminator should be method-specific (computed from the component name + method), similar to how InitializeComponent uses component.GetMethodDiscriminator("initialize") in its bundled overload. This hardcoded value likely only works for standalone components.
Consider adding a discriminator parameter or deriving it from the component. Verify whether delegation instructions for bundled components require component-specific discriminators by checking the bundle implementation in Rust.
🏁 Script executed:
#!/bin/bash
# Description: Search for delegation discriminator handling in Rust bundle code
rg -n "delegate.*discriminator" --type rust -C 3Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for dynamic discriminator usage in C# client
rg -n "GetMethodDiscriminator" --type cs -C3
# Locate and view DelegateComponent definitions
rg -n "public static TransactionInstruction DelegateComponent" -C5 --type cs clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.cs
sed -n '1,200p' clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.csLength of output: 8980
Replace hard-coded discriminator in DelegateComponent with component.GetMethodDiscriminator("delegate").
Both overloads in DelegateComponent.cs should mirror InitializeComponent (and DestroyComponent) by calling component.GetMethodDiscriminator("delegate") (and validating its length) instead of using the fixed [90,147,75,178,85,88,4,137] to ensure correct discriminators for bundled components.
🤖 Prompt for AI Agents
In clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.cs
around line 31, replace the hard-coded byte[] discriminator = new byte[] { 90,
147, 75, 178, 85, 88, 4, 137 }; with logic that calls
component.GetMethodDiscriminator("delegate"), validate the returned array length
is 8 (throw or handle if not), and use that value for the discriminator; mirror
the same pattern used by InitializeComponent/DestroyComponent so bundled
components resolve correct discriminators dynamically and safely.
| // discriminator[8] + commitFrequencyMs[u32 le] + validator[COption<Pubkey>] + pdaSeeds[Vec<Bytes>] | ||
| byte[] discriminator = new byte[] { 90, 147, 75, 178, 85, 88, 4, 137 }; | ||
| uint commitFrequencyMs = 0; | ||
| byte[] commitFrequencyBytes = BitConverter.GetBytes(commitFrequencyMs); // little-endian on most platforms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use explicit little-endian conversion.
BitConverter.GetBytes() uses the system's native endianness, which is not guaranteed to be little-endian. Solana programs expect little-endian byte order.
Apply this diff to ensure consistent little-endian encoding:
- byte[] commitFrequencyBytes = BitConverter.GetBytes(commitFrequencyMs); // little-endian on most platforms
+ byte[] commitFrequencyBytes = BitConverter.GetBytes(commitFrequencyMs);
+ if (!BitConverter.IsLittleEndian) Array.Reverse(commitFrequencyBytes);🤖 Prompt for AI Agents
In clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.cs
around line 33, the code uses BitConverter.GetBytes(commitFrequencyMs) which
relies on system endianness; change this to always produce little-endian bytes
by writing the int to a 4-byte span with BinaryPrimitives.WriteInt32LittleEndian
or by calling BitConverter.GetBytes and reversing the array when
BitConverter.IsLittleEndian is false so the resulting byte[] is guaranteed
little-endian for Solana.
| public static async Task<DelegateComponentInstruction> DelegateComponent(PublicKey payer, PublicKey entity, Component component, string seed = "") { | ||
| var account = WorldProgram.FindComponentPda(component.Program, entity, component.Seeds(seed)); | ||
| var bufferPda = WorldProgram.FindBufferPda(account, component.Program); | ||
| var delegationRecord = WorldProgram.FindDelegationProgramPda("delegation", account); | ||
| var delegationMetadata = WorldProgram.FindDelegationProgramPda("delegation-metadata", account); | ||
|
|
||
| byte[] discriminator = new byte[] { 90, 147, 75, 178, 85, 88, 4, 137 }; | ||
| uint commitFrequencyMs = 0; | ||
| byte[] commitFrequencyBytes = BitConverter.GetBytes(commitFrequencyMs); | ||
| byte[] validatorNoneTag = new byte[] { 0 }; | ||
|
|
||
| var data = Concat(discriminator, commitFrequencyBytes, validatorNoneTag); | ||
|
|
||
| TransactionInstruction instruction = new TransactionInstruction() { | ||
| ProgramId = component.Program, | ||
| Keys = new List<AccountMeta>() { | ||
| AccountMeta.ReadOnly(payer, true), | ||
| AccountMeta.ReadOnly(entity, false), | ||
| AccountMeta.Writable(account, false), | ||
| AccountMeta.ReadOnly(component.Program, false), | ||
| AccountMeta.Writable(bufferPda, false), | ||
| AccountMeta.Writable(delegationRecord, false), | ||
| AccountMeta.Writable(delegationMetadata, false), | ||
| AccountMeta.ReadOnly(WorldProgram.DelegationProgram, false), | ||
| AccountMeta.ReadOnly(SystemProgram.ProgramIdKey, false), | ||
| }, | ||
| Data = data, | ||
| }; | ||
| return new DelegateComponentInstruction() { | ||
| Pda = account, | ||
| Instruction = instruction, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add parameter validation for bundled component overload.
Unlike InitializeComponent, this method doesn't validate the component parameter. The component could be null or have an invalid discriminator.
Apply this diff to add validation consistent with InitializeComponent:
public static async Task<DelegateComponentInstruction> DelegateComponent(PublicKey payer, PublicKey entity, Component component, string seed = "") {
+ if (component is null) throw new ArgumentNullException(nameof(component));
var account = WorldProgram.FindComponentPda(component.Program, entity, component.Seeds(seed));📝 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.
| public static async Task<DelegateComponentInstruction> DelegateComponent(PublicKey payer, PublicKey entity, Component component, string seed = "") { | |
| var account = WorldProgram.FindComponentPda(component.Program, entity, component.Seeds(seed)); | |
| var bufferPda = WorldProgram.FindBufferPda(account, component.Program); | |
| var delegationRecord = WorldProgram.FindDelegationProgramPda("delegation", account); | |
| var delegationMetadata = WorldProgram.FindDelegationProgramPda("delegation-metadata", account); | |
| byte[] discriminator = new byte[] { 90, 147, 75, 178, 85, 88, 4, 137 }; | |
| uint commitFrequencyMs = 0; | |
| byte[] commitFrequencyBytes = BitConverter.GetBytes(commitFrequencyMs); | |
| byte[] validatorNoneTag = new byte[] { 0 }; | |
| var data = Concat(discriminator, commitFrequencyBytes, validatorNoneTag); | |
| TransactionInstruction instruction = new TransactionInstruction() { | |
| ProgramId = component.Program, | |
| Keys = new List<AccountMeta>() { | |
| AccountMeta.ReadOnly(payer, true), | |
| AccountMeta.ReadOnly(entity, false), | |
| AccountMeta.Writable(account, false), | |
| AccountMeta.ReadOnly(component.Program, false), | |
| AccountMeta.Writable(bufferPda, false), | |
| AccountMeta.Writable(delegationRecord, false), | |
| AccountMeta.Writable(delegationMetadata, false), | |
| AccountMeta.ReadOnly(WorldProgram.DelegationProgram, false), | |
| AccountMeta.ReadOnly(SystemProgram.ProgramIdKey, false), | |
| }, | |
| Data = data, | |
| }; | |
| return new DelegateComponentInstruction() { | |
| Pda = account, | |
| Instruction = instruction, | |
| }; | |
| } | |
| public static async Task<DelegateComponentInstruction> DelegateComponent(PublicKey payer, PublicKey entity, Component component, string seed = "") { | |
| if (component is null) throw new ArgumentNullException(nameof(component)); | |
| var account = WorldProgram.FindComponentPda(component.Program, entity, component.Seeds(seed)); | |
| var bufferPda = WorldProgram.FindBufferPda(account, component.Program); | |
| var delegationRecord = WorldProgram.FindDelegationProgramPda("delegation", account); | |
| var delegationMetadata = WorldProgram.FindDelegationProgramPda("delegation-metadata", account); | |
| byte[] discriminator = new byte[] { 90, 147, 75, 178, 85, 88, 4, 137 }; | |
| uint commitFrequencyMs = 0; | |
| byte[] commitFrequencyBytes = BitConverter.GetBytes(commitFrequencyMs); | |
| byte[] validatorNoneTag = new byte[] { 0 }; | |
| var data = Concat(discriminator, commitFrequencyBytes, validatorNoneTag); | |
| TransactionInstruction instruction = new TransactionInstruction() { | |
| ProgramId = component.Program, | |
| Keys = new List<AccountMeta>() { | |
| AccountMeta.ReadOnly(payer, true), | |
| AccountMeta.ReadOnly(entity, false), | |
| AccountMeta.Writable(account, false), | |
| AccountMeta.ReadOnly(component.Program, false), | |
| AccountMeta.Writable(bufferPda, false), | |
| AccountMeta.Writable(delegationRecord, false), | |
| AccountMeta.Writable(delegationMetadata, false), | |
| AccountMeta.ReadOnly(WorldProgram.DelegationProgram, false), | |
| AccountMeta.ReadOnly(SystemProgram.ProgramIdKey, false), | |
| }, | |
| Data = data, | |
| }; | |
| return new DelegateComponentInstruction() { | |
| Pda = account, | |
| Instruction = instruction, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.cs
around lines 63 to 95, add the same parameter validation used in
InitializeComponent: check that component is not null and throw
ArgumentNullException if it is, validate that component.Discriminator is not
null and has the expected length (8 bytes) and throw ArgumentException if
invalid, and optionally validate component.Program is set (throw
ArgumentException if default/empty); perform these checks at the top of the
method before computing PDAs so the method fails fast on bad input.
| let mut sha256 = Sha256::new(); | ||
| sha256.update(format!("account:{}", name_str).as_bytes()); | ||
| let discriminator = sha256.finalize()[0..8].to_vec(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify cross-language discriminator consistency.
Ensure that the TypeScript and C# implementations compute discriminators using the same "account:{name}" format and SHA-256 algorithm. Inconsistencies could lead to routing failures in bundled deployments.
Run the following script to compare discriminator implementations across languages:
🏁 Script executed:
#!/bin/bash
# Description: Verify discriminator computation format across Rust, TypeScript, and C#
echo "=== Rust implementation (current file) ==="
rg -n -A2 'sha256.update.*format!' crates/bolt-lang/attribute/component-deserialize/src/lib.rs
echo -e "\n=== TypeScript GetDiscriminator implementation ==="
rg -n -C5 'GetDiscriminator|createHash.*account:' clients/typescript/src
echo -e "\n=== C# GetDiscriminator implementation ==="
rg -n -C5 'GetDiscriminator|ComputeHash.*account:' clients/csharp/srcLength of output: 2963
🏁 Script executed:
#!/bin/bash
echo "=== clients/typescript/src/index.ts ==="
sed -n '1,100p' clients/typescript/src/index.ts
echo -e "\n=== All TS calls to GetDiscriminator ==="
rg -n 'GetDiscriminator' -C2 clients/typescript/src
echo -e "\n=== TS calls including \"account:\" prefix ==="
rg -n 'GetDiscriminator.*\"account:' -C2 clients/typescript/src || echo "No direct calls with \"account:\""
echo -e "\n=== Occurrences of string literal \"account:\" in TS ==="
rg -n '\"account:' -C1 clients/typescript/src || echo "No string literal \"account:\" in TS"
echo -e "\n=== Listing .cs files in repo ==="
fd -e cs || echo "No .cs files found"
echo -e "\n=== Search for ComputeHash in repo ==="
rg -n 'ComputeHash' -C2 .Length of output: 6309
Align discriminator prefixes across Rust, TypeScript, and C#
Rust (crates/bolt-lang/attribute/component-deserialize/src/lib.rs:42) prefixes the name with "account:" before hashing, whereas TypeScript’s GetDiscriminator(name) and C#’s SHA256.ComputeHash(nameBytes) both hash the raw name string. They will produce different 8-byte discriminators. Update TS and C# to prepend "account:" to the name (e.g. GetDiscriminator(\account:${name}`)andEncoding.UTF8.GetBytes($"account:{name}")`), or remove the prefix in Rust so all three implementations compute identical values.
| module.content.as_mut().map(|(brace, items)| { | ||
| items.extend( | ||
| vec![initialize_fn, initialize_struct, destroy_fn, destroy_struct] | ||
| .into_iter() | ||
| .map(|item| syn::parse2(item).expect("Failed to parse generate initialize and destroy item")) | ||
| .collect::<Vec<_>>(), | ||
| ); | ||
|
|
||
| let modified_items: Vec<syn::Item> = items | ||
| .iter_mut() | ||
| .map(|item| match item.clone() { | ||
| syn::Item::Struct(mut struct_item) | ||
| if struct_item.ident == "Apply" || struct_item.ident == "ApplyWithSession" => | ||
| { | ||
| modify_apply_struct(&mut struct_item); | ||
| syn::Item::Struct(struct_item) | ||
| } | ||
| _ => item.clone(), | ||
| }) | ||
| .collect(); | ||
| (brace, modified_items) | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Apply struct modifications are never applied.
The modified_items vector is computed but never assigned back to items. The closure returns (brace, modified_items), but the .map() result is discarded. Since line 44 clones items, modifications to struct_item in line 48 affect only the clones, not the original module items.
Apply this diff to fix the issue:
- module.content.as_mut().map(|(brace, items)| {
+ if let Some((_, items)) = module.content.as_mut() {
items.extend(
vec![initialize_fn, initialize_struct, destroy_fn, destroy_struct]
.into_iter()
.map(|item| syn::parse2(item).expect("Failed to parse generate initialize and destroy item"))
.collect::<Vec<_>>(),
);
- let modified_items: Vec<syn::Item> = items
- .iter_mut()
- .map(|item| match item.clone() {
- syn::Item::Struct(mut struct_item)
- if struct_item.ident == "Apply" || struct_item.ident == "ApplyWithSession" =>
- {
- modify_apply_struct(&mut struct_item);
- syn::Item::Struct(struct_item)
- }
- _ => item.clone(),
- })
- .collect();
- (brace, modified_items)
- });
+ for item in items.iter_mut() {
+ if let syn::Item::Struct(struct_item) = item {
+ if struct_item.ident == "Apply" || struct_item.ident == "ApplyWithSession" {
+ modify_apply_struct(struct_item);
+ }
+ }
+ }
+ }📝 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.
| module.content.as_mut().map(|(brace, items)| { | |
| items.extend( | |
| vec![initialize_fn, initialize_struct, destroy_fn, destroy_struct] | |
| .into_iter() | |
| .map(|item| syn::parse2(item).expect("Failed to parse generate initialize and destroy item")) | |
| .collect::<Vec<_>>(), | |
| ); | |
| let modified_items: Vec<syn::Item> = items | |
| .iter_mut() | |
| .map(|item| match item.clone() { | |
| syn::Item::Struct(mut struct_item) | |
| if struct_item.ident == "Apply" || struct_item.ident == "ApplyWithSession" => | |
| { | |
| modify_apply_struct(&mut struct_item); | |
| syn::Item::Struct(struct_item) | |
| } | |
| _ => item.clone(), | |
| }) | |
| .collect(); | |
| (brace, modified_items) | |
| }); | |
| if let Some((_, items)) = module.content.as_mut() { | |
| items.extend( | |
| vec![initialize_fn, initialize_struct, destroy_fn, destroy_struct] | |
| .into_iter() | |
| .map(|item| syn::parse2(item).expect("Failed to parse generate initialize and destroy item")) | |
| .collect::<Vec<_>>(), | |
| ); | |
| for item in items.iter_mut() { | |
| if let syn::Item::Struct(struct_item) = item { | |
| if struct_item.ident == "Apply" || struct_item.ident == "ApplyWithSession" { | |
| modify_apply_struct(struct_item); | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In crates/bolt-lang/attribute/src/component/generate/program.rs around lines
34–55, the code computes modified_items but never writes it back to the module
items (the .map() result is discarded and the modifications only affect clones).
Replace the .map() usage with logic that updates the original items in-place:
iterate/mutate the existing items (or assign modified_items back to the items
variable) so the modified_items replace the original items in module.content;
avoid working only on clones so modify_apply_struct changes are persisted.
| let undelegate_fn = quote! { | ||
| #[automatically_derived] | ||
| pub fn undelegate(ctx: Context<Undelegate>) -> Result<()> { | ||
| ::bolt_lang::commit_and_undelegate_accounts( | ||
| &ctx.accounts.payer, | ||
| vec![&ctx.accounts.delegated_account.to_account_info()], | ||
| &ctx.accounts.magic_context, | ||
| &ctx.accounts.magic_program, | ||
| )?; | ||
| Ok(()) | ||
| } | ||
| }; | ||
|
|
||
| let undelegate_struct = quote! { | ||
| #[automatically_derived] | ||
| #[derive(Accounts)] | ||
| pub struct Undelegate<'info> { | ||
| #[account(mut)] | ||
| pub payer: Signer<'info>, | ||
| #[account(mut)] | ||
| /// CHECK: The delegated component | ||
| pub delegated_account: AccountInfo<'info>, | ||
| #[account(mut, address = ::bolt_lang::MAGIC_CONTEXT_ID)] | ||
| /// CHECK:` | ||
| pub magic_context: AccountInfo<'info>, | ||
| #[account()] | ||
| /// CHECK:` | ||
| pub magic_program: Program<'info, MagicProgram> | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify MagicProgram availability in generated code context.
Line 180 references Program<'info, MagicProgram>, but MagicProgram is not imported in this file. While the generated code will be injected into a program module, ensure that MagicProgram is accessible in the target context (either through imports in the target module or via fully-qualified path like ::bolt_lang::MagicProgram).
Run this script to verify if MagicProgram is properly exported and accessible:
🏁 Script executed:
#!/bin/bash
# Description: Check if MagicProgram is defined and exported in bolt_lang or related crates
# Search for MagicProgram struct or type definition
rg -n -A 3 'pub\s+(struct|type)\s+MagicProgram'
# Search for MagicProgram in bolt_lang exports
rg -n -A 3 'pub use.*MagicProgram' crates/bolt-lang/src/lib.rsLength of output: 276
Qualify MagicProgram in generated code
MagicProgram is re-exported by this crate (crates/bolt-lang/src/lib.rs:29). Update the generated struct field to:
-pub magic_program: Program<'info, MagicProgram>
+pub magic_program: Program<'info, ::bolt_lang::MagicProgram>📝 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 undelegate_fn = quote! { | |
| #[automatically_derived] | |
| pub fn undelegate(ctx: Context<Undelegate>) -> Result<()> { | |
| ::bolt_lang::commit_and_undelegate_accounts( | |
| &ctx.accounts.payer, | |
| vec![&ctx.accounts.delegated_account.to_account_info()], | |
| &ctx.accounts.magic_context, | |
| &ctx.accounts.magic_program, | |
| )?; | |
| Ok(()) | |
| } | |
| }; | |
| let undelegate_struct = quote! { | |
| #[automatically_derived] | |
| #[derive(Accounts)] | |
| pub struct Undelegate<'info> { | |
| #[account(mut)] | |
| pub payer: Signer<'info>, | |
| #[account(mut)] | |
| /// CHECK: The delegated component | |
| pub delegated_account: AccountInfo<'info>, | |
| #[account(mut, address = ::bolt_lang::MAGIC_CONTEXT_ID)] | |
| /// CHECK:` | |
| pub magic_context: AccountInfo<'info>, | |
| #[account()] | |
| /// CHECK:` | |
| pub magic_program: Program<'info, MagicProgram> | |
| } | |
| }; | |
| let undelegate_struct = quote! { | |
| #[automatically_derived] | |
| #[derive(Accounts)] | |
| pub struct Undelegate<'info> { | |
| #[account(mut)] | |
| pub payer: Signer<'info>, | |
| #[account(mut)] | |
| /// CHECK: The delegated component | |
| pub delegated_account: AccountInfo<'info>, | |
| #[account(mut, address = ::bolt_lang::MAGIC_CONTEXT_ID)] | |
| /// CHECK:` | |
| pub magic_context: AccountInfo<'info>, | |
| #[account()] | |
| /// CHECK:` | |
| pub magic_program: Program<'info, ::bolt_lang::MagicProgram> | |
| } | |
| }; |
🤖 Prompt for AI Agents
In crates/bolt-lang/attribute/src/delegate/mod.rs around lines 153 to 182, the
generated struct uses an unqualified MagicProgram type in the Program<'info,
MagicProgram> field; update the generated code to fully qualify the type as
::bolt_lang::MagicProgram so the macro output references the re-exported symbol
unambiguously (i.e., replace the unqualified MagicProgram with
::bolt_lang::MagicProgram in the magic_program field's type).
| panic!( | ||
| "Could not find the component bundle: {} in the module", | ||
| extractor.context_struct_name.unwrap() | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve error handling in panic path.
If both field_count and context_struct_name are None, the panic will occur inside the error message formatting due to the unwrap() on line 79, making debugging more difficult.
Apply this diff to ensure a clear error message:
} else {
panic!(
"Could not find the component bundle: {} in the module",
- extractor.context_struct_name.unwrap()
+ extractor.context_struct_name.unwrap_or_else(|| "unknown".to_string())
);
}📝 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.
| panic!( | |
| "Could not find the component bundle: {} in the module", | |
| extractor.context_struct_name.unwrap() | |
| ); | |
| } | |
| } else { | |
| panic!( | |
| "Could not find the component bundle: {} in the module", | |
| extractor.context_struct_name.unwrap_or_else(|| "unknown".to_string()) | |
| ); | |
| } |
🤖 Prompt for AI Agents
In crates/bolt-lang/attribute/src/system/mod.rs around lines 77 to 81, the panic
currently calls unwrap() on extractor.context_struct_name which will itself
panic if that field is None; change the panic to avoid unwrap by formatting a
clear error that includes both extractor.field_count and
extractor.context_struct_name (showing e.g. "None" when absent). Replace the
unwrap usage with a safe display (match/map_or/unwrap_or) or build the message
using Option::as_deref/unwrap_or("<none>") so the panic text always prints both
values and gives a clear, non-panicking error message.
| if extractor.field_count.is_none() { | ||
| panic!( | ||
| "Could not find the component bundle: {} in the module", | ||
| extractor.context_struct_name.unwrap_or_default() | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve error handling in panic path.
Similar to the process function, using unwrap_or_default() on an empty string may not provide a useful error message to the user.
Apply this diff for better error reporting:
if extractor.field_count.is_none() {
panic!(
"Could not find the component bundle: {} in the module",
- extractor.context_struct_name.unwrap_or_default()
+ extractor.context_struct_name.as_deref().unwrap_or("unknown")
);
}📝 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.
| if extractor.field_count.is_none() { | |
| panic!( | |
| "Could not find the component bundle: {} in the module", | |
| extractor.context_struct_name.unwrap_or_default() | |
| ); | |
| } | |
| if extractor.field_count.is_none() { | |
| panic!( | |
| "Could not find the component bundle: {} in the module", | |
| extractor.context_struct_name.as_deref().unwrap_or("unknown") | |
| ); | |
| } |
🤖 Prompt for AI Agents
In crates/bolt-lang/attribute/src/system/mod.rs around lines 90 to 95, the panic
message uses extractor.context_struct_name.unwrap_or_default(), which can
produce an empty/ambiguous name; instead, surface a meaningful identifier by
avoiding unwrap_or_default — retrieve the context_struct_name with a clear
fallback like "<unknown component>" (or use expect with a descriptive message)
and include that in the panic so the error message is informative; update the
panic call to use the non-empty fallback or expect variant and keep the
surrounding panic text unchanged.
| if let Some(suffix) = name_suffix { | ||
| let pascal = suffix.to_pascal_case(); | ||
| let new_components_ident = Ident::new(&format!("{}Components", pascal), Span::call_site()); | ||
| let new_bumps_ident = Ident::new(&format!("{}ComponentsBumps", pascal), Span::call_site()); | ||
|
|
||
| struct SystemRename { | ||
| new_components: Ident, | ||
| new_bumps: Ident, | ||
| } | ||
| impl VisitMut for SystemRename { | ||
| fn visit_item_struct_mut(&mut self, i: &mut ItemStruct) { | ||
| if i.ident == "Components" { | ||
| i.ident = self.new_components.clone(); | ||
| } else if i.ident == "ComponentsBumps" { | ||
| i.ident = self.new_bumps.clone(); | ||
| } | ||
| syn::visit_mut::visit_item_struct_mut(self, i); | ||
| } | ||
| fn visit_type_path_mut(&mut self, i: &mut TypePath) { | ||
| for seg in i.path.segments.iter_mut() { | ||
| if seg.ident == "Components" { | ||
| seg.ident = self.new_components.clone(); | ||
| } else if seg.ident == "ComponentsBumps" { | ||
| seg.ident = self.new_bumps.clone(); | ||
| } | ||
| } | ||
| syn::visit_mut::visit_type_path_mut(self, i); | ||
| } | ||
| fn visit_expr_path_mut(&mut self, i: &mut syn::ExprPath) { | ||
| if let Some(seg) = i.path.segments.last_mut() { | ||
| if seg.ident == "Components" { | ||
| seg.ident = self.new_components.clone(); | ||
| } else if seg.ident == "ComponentsBumps" { | ||
| seg.ident = self.new_bumps.clone(); | ||
| } | ||
| } | ||
| syn::visit_mut::visit_expr_path_mut(self, i); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider extracting inline visitor structs.
The SystemRename and ExecRename visitor structs defined inline make this function harder to read and test. Consider extracting them as separate private structs at module level.
This would improve code organization and make the function more focused on its primary transformation logic.
🤖 Prompt for AI Agents
crates/bolt-lang/attribute/src/system/mod.rs lines 105-143: the inline visitor
structs SystemRename and ExecRename should be moved out of the function and
defined as private module-level structs to improve readability and testability;
create private structs (e.g., struct SystemRename { new_components: Ident,
new_bumps: Ident } and similarly for ExecRename with whatever fields it needs),
move their VisitMut impl blocks to the module level, keep their logic unchanged,
ensure necessary imports (syn::visit_mut::VisitMut, Ident, TypePath, ItemStruct,
ExprPath) are visible at module scope, update the local code to instantiate
these structs with the new idents and call visit_mut as before, and run
tests/formatting to confirm no API/visibility regressions.
docs/REPORT.md
Outdated
| bar [14428,22876,31384,39897,48603,57176,65796,74635,83475,92178] | ||
| bar [5940,10903,15896,20868,25841,30816,35811,40997,45992,50986] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Data update looks valid; add series labels for clarity (legend).
Arrays match x-axis length and stay within the y-axis range. To improve readability, label the series so a legend renders and the mapping is unambiguous.
Proposed labels (please verify ordering/mapping):
- bar [14428,22876,31384,39897,48603,57176,65796,74635,83475,92178]
- bar [5940,10903,15896,20868,25841,30816,35811,40997,45992,50986]
+ bar "Bundle" [14428,22876,31384,39897,48603,57176,65796,74635,83475,92178]
+ bar "Standalone" [5940,10903,15896,20868,25841,30816,35811,40997,45992,50986]Optional: consider y-axis starting at 0 for easier visual comparison.
📝 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.
| bar [14428,22876,31384,39897,48603,57176,65796,74635,83475,92178] | |
| bar [5940,10903,15896,20868,25841,30816,35811,40997,45992,50986] | |
| bar "Bundle" [14428,22876,31384,39897,48603,57176,65796,74635,83475,92178] | |
| bar "Standalone" [5940,10903,15896,20868,25841,30816,35811,40997,45992,50986] |
🤖 Prompt for AI Agents
In docs/REPORT.md around lines 7 to 8, the two "bar" data series lack labels
making the legend and mapping ambiguous; update the lines to include explicit
series labels (e.g., "bar_series_1" and "bar_series_2" or the appropriate
domain-specific names) in the same order as the arrays to ensure the legend maps
correctly, verify the label-to-array ordering matches intended visualization,
and optionally set the chart y-axis to start at 0 for clearer comparison.
bd3ddde to
8e0ca48
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 57
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (15)
scripts/test-publish.sh (2)
1-9: Harden the script: strict mode, env‑configurable DRY_RUN, clean conditionals.Add shebang and strict flags; default DRY_RUN from env; normalize conditionals; also remove trailing spaces on Line 9.
Apply:
+#!/usr/bin/env bash +set -euo pipefail -DRY_RUN="true" -DRY_RUN_FLAG="" -if [ "${DRY_RUN}" = "true" ]; then -DRY_RUN_FLAG="--dry-run" -fi +DRY_RUN="${DRY_RUN:-true}" +DRY_RUN_FLAG="" +if [[ "${DRY_RUN}" == "true" ]]; then + DRY_RUN_FLAG="--dry-run" +fi -if [ "${DRY_RUN}" = "true" ]; then -NO_VERIFY_FLAG="--no-verify" -fi +NO_VERIFY_FLAG="" +if [[ "${DRY_RUN}" == "true" ]]; then + NO_VERIFY_FLAG="--no-verify" +fi
11-23: Optional: safer arg handling with arrays.Avoid word‑splitting and quoting issues; build args and exec cargo.
Example (outside current lines for brevity):
args=(+nightly publish -Zpackage-workspace) [[ -n "${DRY_RUN_FLAG}" ]] && args+=("${DRY_RUN_FLAG}") [[ -n "${NO_VERIFY_FLAG}" ]] && args+=("${NO_VERIFY_FLAG}") args+=( -p world -p bolt-cli -p bolt-lang -p bolt-utils -p bolt-attribute-bolt-arguments -p bolt-attribute-bolt-bundle -p bolt-attribute-bolt-component -p bolt-attribute-bolt-component-deserialize -p bolt-attribute-bolt-component-id -p bolt-attribute-bolt-extra-accounts -p bolt-attribute-bolt-system -p bolt-attribute-bolt-system-input ) cargo "${args[@]}"crates/bolt-lang/utils/src/lib.rs (1)
5-16: Useparse_quote!with absolutebolt_langpath and add idempotency
- Replace the manual
Fieldconstruction with:let bolt_metadata_field: syn::Field = syn::parse_quote! { pub bolt_metadata: ::bolt_lang::BoltMetadata };- Before inserting, guard against duplicates and unsupported shapes:
if let syn::Data::Struct(ref mut data) = input.data { if let syn::Fields::Named(ref mut fields) = data.fields { if fields.named.iter().any(|f| f.ident.as_deref() == Some("bolt_metadata")) { return; } } else { abort!(input.ident, "#[bundle] requires a struct with named fields"); } }.github/workflows/publish-bolt-crates.yml (3)
39-46: Unify actions/cache versions.
You're mixing v3 and v4; standardize on v4 for consistency.- - name: Cache node dependencies - uses: actions/cache@v3 + - name: Cache node dependencies + uses: actions/cache@v4
101-105: Step name references matrix without a matrix.
Rename for clarity.- - name: Use Node ${{ matrix.node }} + - name: Use Node 21
131-142: Remove stale program keypair echoes in .github/workflows/publish-bolt-crates.yml (lines 131–142)
Standalone program keypairs (bolt_component, bolt_system, component_position, component_velocity, system_apply_velocity, system_fly, system_simple_movement) are not referenced anywhere in the codebase—drop their secrets and correspondingechocommands to prevent secret drift and avoid failures.crates/bolt-lang/Cargo.toml (1)
19-27: Remove stalebolt-attribute-bolt-programentry from root Cargo.toml
The crate-levelcrates/bolt-lang/Cargo.tomlno longer references any delegate/program attributes, but the workspace root Cargo.toml still hasbolt-attribute-bolt-program = { path = "crates/bolt-lang/attribute/bolt-program", version = "=0.2.6" }Please delete that line (and any
bolt-attribute-bolt-delegateif present).crates/bolt-lang/attribute/component-deserialize/src/lib.rs (2)
61-63: Bounds check before slicing buffer
let mut data: &[u8] = &buf[8..];will panic ifbuf.len() < 8. Return a proper error instead.- fn try_deserialize_unchecked(buf: &mut &[u8]) -> bolt_lang::Result<Self> { - let mut data: &[u8] = &buf[8..]; + fn try_deserialize_unchecked(buf: &mut &[u8]) -> bolt_lang::Result<Self> { + if buf.len() < 8 { + return Err(bolt_lang::AccountDidNotDeserializeErrorCode.into()); + } + let mut data: &[u8] = &buf[8..]; bolt_lang::AnchorDeserialize::deserialize(&mut data) .map_err(|_| bolt_lang::AccountDidNotDeserializeErrorCode.into()) }
69-73: Use fully-qualified Result type
Result<()>is unresolved in generated context. Usebolt_lang::Result<()>for consistency with adjacent code.- fn try_serialize<W>(&self, _writer: &mut W) -> Result<()> { + fn try_serialize<W>(&self, _writer: &mut W) -> bolt_lang::Result<()> { Ok(()) }crates/bolt-cli/src/lib.rs (1)
321-329: Include bundles path in new workspace
init()adds components/systems to workspace but not bundles. Without this,bolt bundlescaffolds underprograms-ecs/bundles/*won’t be built by Cargo/Anchor.cfg.workspace .members .push("programs-ecs/systems/*".to_owned()); + cfg.workspace + .members + .push("programs-ecs/bundles/*".to_owned());clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DestroyComponent.cs (1)
46-65: Missing DestroyComponentWithDiscriminator overload
The diff callsWorldProgram.DestroyComponentWithDiscriminator, but no such overload exists. You must update the generated API (e.g. in WorldProgram.Generated.cs alongside InitializeComponentWithDiscriminator) to include a DestroyComponentWithDiscriminator method before invoking it here.crates/bolt-lang/attribute/system-input/src/lib.rs (2)
31-62: Name-collision risk:use … as #field_typecan shadow real types (e.g.,Position) in bundles.Aliasing to the original field type name will clash when that type already exists in the same module (common in bundles). Prefer a unique private alias per field (e.g.,
__bolt_component_Position) and reference that in the generated struct; or replace the field’s type inline with the fully-qualifiedbolt_types::ComponentXwhen#[component_id("X")]is present.I can supply a targeted patch to switch to unique aliases and update
transformed_fields; confirm which approach you prefer.
124-130: Reorder generated component imports before the struct and impl
Ensures alias imports (e.g. for#[component_id]in examples/system-simple-movement/src/lib.rs:23) are in scope before use.- let output = quote! { - #output_struct - #output_impl - #output_trait_implementation - #(#components_imports)* - }; + let output = quote! { + #(#components_imports)* + #output_struct + #output_impl + #output_trait_implementation + };crates/bolt-lang/src/lib.rs (1)
49-55: Avoid intermediate UTF-8 allocation inparse_args.Deserialize directly from bytes to cut a copy and a panic site; bubble errors via
Resultif you prefer not to panic on-chain.Minimal change:
-pub fn parse_args<T: serde::de::DeserializeOwned>(args_p: &[u8]) -> T { - let args_string = str::from_utf8(args_p).expect("Failed to convert to string"); - let args: T = serde_json::from_str(args_string) - .unwrap_or_else(|_| panic!("Failed to deserialize args: {:?}", args_string)); - args -} +pub fn parse_args<T: serde::de::DeserializeOwned>(args_p: &[u8]) -> T { + serde_json::from_slice(args_p) + .unwrap_or_else(|_| panic!("Failed to deserialize args (len={}): {:?}", args_p.len(), args_p)) +}clients/typescript/src/world/transactions.ts (1)
568-569: Make ApplySystemComponent accept domain objectsLoop expects
Component.from(...), but the type only allows PublicKey.-interface ApplySystemComponent { - componentId: PublicKey; +interface ApplySystemComponent { + componentId: PublicKey | Component; seed?: string; }
♻️ Duplicate comments (30)
crates/bolt-lang/attribute/src/delegate/mod.rs (4)
49-51: PDA seed construction is incorrect for bundles; also avoid temporary to_bytes() refs.Use [name_bytes, entity_key] seeds; drop component::seed(); reference a stable buffer.
- let component_matches = components.iter().map(|(component, name)| quote! { - #component::DISCRIMINATOR => &[#component::seed(), #name.as_bytes(), &ctx.accounts.entity.key().to_bytes()] - }).collect::<Vec<_>>(); + let component_matches = components.iter().map(|(component, name)| quote! { + d if d == #component::DISCRIMINATOR => &[#name.as_bytes(), entity_key_bytes.as_ref()] + }).collect::<Vec<_>>();Additionally apply the companion changes below (lines 55-61) to define entity_key_bytes and use a guard match.
63-72: Align field name with DelegateAccounts.pda and struct field.Rename DelegateInput.account → pda, and update usage for clarity/consistency.
let del_accounts = ::bolt_lang::DelegateAccounts { payer: &ctx.accounts.payer, - pda: &ctx.accounts.account, + pda: &ctx.accounts.pda, owner_program: &ctx.accounts.owner_program, buffer: &ctx.accounts.buffer, delegation_record: &ctx.accounts.delegation_record, delegation_metadata: &ctx.accounts.delegation_metadata, delegation_program: &ctx.accounts.delegation_program, system_program: &ctx.accounts.system_program, };
86-112: Rename field to pda; remove stray backticks; avoid empty #[account()].
- account → pda.
- Fix "/// CHECK:`" typos.
- Remove redundant #[account()] when no constraints are specified.
pub struct DelegateInput<'info> { pub payer: Signer<'info>, - #[account()] + #[account()] pub entity: Account<'info, Entity>, /// CHECK: #[account(mut)] - pub account: AccountInfo<'info>, - /// CHECK:` + pub pda: AccountInfo<'info>, + /// CHECK: pub owner_program: AccountInfo<'info>, /// CHECK: #[account(mut)] pub buffer: AccountInfo<'info>, - /// CHECK:` + /// CHECK: #[account(mut)] pub delegation_record: AccountInfo<'info>, - /// CHECK:` + /// CHECK: #[account(mut)] pub delegation_metadata: AccountInfo<'info>, - /// CHECK:` + /// CHECK: pub delegation_program: AccountInfo<'info>, - /// CHECK:` + /// CHECK: pub system_program: AccountInfo<'info>, }Note: Consider dropping empty #[account()] entirely on fields that are plain AccountInfo with CHECK docs.
166-182: Fully qualify MagicProgram and fix CHECK typos.Qualify via ::bolt_lang to avoid missing imports; clean docs.
pub struct Undelegate<'info> { #[account(mut)] pub payer: Signer<'info>, #[account(mut)] /// CHECK: The delegated component pub delegated_account: AccountInfo<'info>, #[account(mut, address = ::bolt_lang::MAGIC_CONTEXT_ID)] - /// CHECK:` + /// CHECK: pub magic_context: AccountInfo<'info>, #[account()] - /// CHECK:` - pub magic_program: Program<'info, MagicProgram> + /// CHECK: + pub magic_program: Program<'info, ::bolt_lang::MagicProgram> }docs/REPORT.md (1)
7-8: Add series labels so the legend is unambiguous.The two
barseries are unlabeled; add labels (e.g., “Bundle”, “Standalone”) so charts render a clear legend.- bar [13110,20640,28678,37169,46306,55754,65697,76307,87375,98746] - bar [4581,8633,13163,18120,23526,29382,35706,42669,49889,57556] + bar "Bundle" [13110,20640,28678,37169,46306,55754,65697,76307,87375,98746] + bar "Standalone" [4581,8633,13163,18120,23526,29382,35706,42669,49889,57556]crates/bolt-lang/attribute/bundle/Cargo.toml (1)
2-3: Improve package description; verify workspace dep availability.
- The description is tautological; make it informative.
- Ensure
bolt-attribute.workspace = trueis satisfied by[workspace.dependencies]in the root.-name = "bolt-attribute-bolt-bundle" -description = "bolt-attribute-bolt-bundle" +name = "bolt-attribute-bolt-bundle" +description = "Bolt bundle attribute for combining components and systems"#!/bin/bash # Confirm workspace provides bolt-attribute rg -nP '(?m)^\[workspace\.dependencies\][\s\S]*\bbolt-attribute\b' -C2 Cargo.tomlAlso applies to: 15-15
examples/bundle/Cargo.toml (2)
3-3: Use workspace-inherited version for consistency.-version = "0.2.5" +version = { workspace = true }
25-25: Prefer workspace dep inheritance for serde (matches other examples).-serde = { version = "1.0", features = ["derive"] } +serde.workspace = truecrates/bolt-cli/src/bundle.rs (1)
6-6: Fix comment: it creates a bundle, not a component.-// Create a new component from the template +// Create a new bundle from the templatecrates/bolt-lang/src/instructions/initialize.rs (1)
6-6: Remove unnecessary Clone bound.
set_inner(T)takes ownership;Clonenot needed.- T: Default + AccountSerialize + AccountDeserialize + BorshDeserialize + Clone, + T: Default + AccountSerialize + AccountDeserialize + BorshDeserialize,crates/bolt-lang/attribute/src/component/generate/program.rs (3)
34-57: Bug: modifications are computed but never applied.
.map(...)result ignored;modified_itemsnot written back.- module.content.as_mut().map(|(brace, items)| { + if let Some((_, items)) = module.content.as_mut() { items.extend( vec![initialize_fn, initialize_struct, destroy_fn, destroy_struct] .into_iter() .map(|item| { syn::parse2(item).expect("Failed to parse generate initialize and destroy item") }) .collect::<Vec<_>>(), ); - - let modified_items: Vec<syn::Item> = items - .iter_mut() - .map(|item| match item.clone() { - syn::Item::Struct(mut struct_item) - if struct_item.ident == "Apply" || struct_item.ident == "ApplyWithSession" => - { - modify_apply_struct(&mut struct_item); - syn::Item::Struct(struct_item) - } - _ => item.clone(), - }) - .collect(); - (brace, modified_items) - }); + for item in items.iter_mut() { + if let syn::Item::Struct(struct_item) = item { + if struct_item.ident == "Apply" || struct_item.ident == "ApplyWithSession" { + modify_apply_struct(struct_item); + } + } + } + }
234-247: Avoid unused return and clone; mutate in place.
Simplify and remove(brace, items.clone())which is discarded.- module.content.as_mut().map(|(brace, items)| { + if let Some((_, items)) = module.content.as_mut() { items.extend( vec![ update_fn, update_struct, update_with_session_fn, update_with_session_struct, ] .into_iter() .map(|item| syn::parse2(item).expect("Failed to parse generate update item")) .collect::<Vec<_>>(), ); - (brace, items.clone()) - }); + }
250-253: Don’t use string contains for type checks.
MatchType::Pathand compare segment ident to “Program”.fn is_expecting_program(field: &Field) -> bool { - field.ty.to_token_stream().to_string().contains("Program") + if let Type::Path(type_path) = &field.ty { + type_path.path.segments.iter().any(|seg| seg.ident == "Program") + } else { + false + } }crates/bolt-lang/attribute/src/component/generate/mod.rs (2)
32-38: Seed must not be empty; fallback to type name.
Prevents empty PDA seeds whencomponent_idis omitted.- let component_id_value = &attributes.component_id; + let seed_value = if attributes.component_id.is_empty() { + input.ident.to_string() + } else { + attributes.component_id.clone() + }; let implementation = quote! { #[automatically_derived] impl ComponentTraits for #name { fn seed() -> &'static [u8] { - #component_id_value.as_bytes() + #seed_value.as_bytes() }Based on learnings
57-61: Exclude bolt_metadata from Init struct and initializer.
Avoid duplicate field/assignment.- let init_struct_fields = fields.named.iter().map(|f| { - let name = &f.ident; - let ty = &f.ty; - quote! { pub #name: #ty } - }); + let init_struct_fields = fields.named.iter().filter_map(|f| { + let name = f.ident.as_ref()?; + if name == "bolt_metadata" { return None; } + let ty = &f.ty; + Some(quote! { pub #name: #ty }) + }); ... - let struct_init_fields = fields.named.iter().map(|f| { - let name = &f.ident; - quote! { #name: init_struct.#name } - }); + let struct_init_fields = fields.named.iter().filter_map(|f| { + let name = f.ident.as_ref()?; + if name == "bolt_metadata" { return None; } + Some(quote! { #name: init_struct.#name }) + });Also applies to: 64-67, 79-81
crates/bolt-lang/attribute/component-deserialize/src/lib.rs (1)
42-44: Re-verify cross-language discriminator prefixYou hash
"account:{TypeName}", which matches Anchor’s convention. Ensure TS/C# do the same when computing discriminators to avoid bundle routing mismatches. Also keep params asVec<u8>in clients, not[u8; 8]. Based on learnings.#!/bin/bash echo "=== Rust discriminators expecting 'account:' prefix ===" rg -n 'account:\{\}' -n crates/bolt-lang/attribute/component-deserialize/src/lib.rs || true echo -e "\n=== TypeScript: verify 'account:' used in discriminator hashing ===" rg -n -C2 'createHash|GetDiscriminator' clients/typescript/src | sed -n '1,200p' rg -n '"account:' clients/typescript/src || echo "TS: no explicit 'account:' literal found" echo -e "\n=== C#: verify 'account:' used in discriminator hashing ===" fd -e cs clients/csharp | sed -n '1,5p' rg -n -C2 'GetDiscriminator|ComputeHash' clients/csharp || true rg -n '"account:' clients/csharp || echo "C#: no explicit 'account:' literal found"clients/csharp/Solana.Unity.Bolt/ECS/Identifier.cs (1)
12-17: Make properties immutable and align nullability with usage.Use init-only and nullable Name to reflect optional identifier names.
- public PublicKey Program { get; set; } + public PublicKey Program { get; init; } @@ - public string Name { get; set; } + public string? Name { get; init; }clients/csharp/Solana.Unity.Bolt.Test/ECSTest.cs (2)
120-129: Use structured deserialization for consistency.Replace manual binary parsing with the generated account deserializer (as done for Position at Line 105) to avoid brittleness.
Suggested change:
- int offset = 8; // skip discriminator - long x = BitConverter.ToInt64(data, offset); offset += 8; - long y = BitConverter.ToInt64(data, offset); offset += 8; - long z = BitConverter.ToInt64(data, offset); offset += 8; - Debug.Assert(1 == x, "X is not equal to 1"); - Debug.Assert(2 == y, "Y is not equal to 2"); - Debug.Assert(3 == z, "Z is not equal to 3"); + var velocity = Velocity.Accounts.Velocity.Deserialize(data); + Debug.Assert(1 == velocity.X, "X is not equal to 1"); + Debug.Assert(2 == velocity.Y, "Y is not equal to 2"); + Debug.Assert(3 == velocity.Z, "Z is not equal to 3");
236-245: Same: prefer the generated deserializer over manual parsing.Use
Velocity.Accounts.Velocity.Deserialize(data)here as well.clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.cs (2)
33-33: Always write little‑endian integers.Guard against big‑endian hosts when building Solana payloads.
- byte[] commitFrequencyBytes = BitConverter.GetBytes(commitFrequencyMs); // little-endian on most platforms + byte[] commitFrequencyBytes = BitConverter.GetBytes(commitFrequencyMs); + if (!BitConverter.IsLittleEndian) Array.Reverse(commitFrequencyBytes);Also applies to: 71-71
63-65: Validatecomponentparameter.Add null checks (and optionally validate
Program), fail fast before PDA derivation.- public static async Task<DelegateComponentInstruction> DelegateComponent(PublicKey payer, PublicKey entity, Component component, string seed = "") { + public static async Task<DelegateComponentInstruction> DelegateComponent(PublicKey payer, PublicKey entity, Component component, string seed = "") { + if (component is null) throw new ArgumentNullException(nameof(component));crates/bolt-lang/attribute/src/common/mod.rs (1)
16-31: Don’t usemapfor side effects; avoid cloning items.Use
if let Some((_brace, items)) = module.content.as_mut()and mutate in place. The(brace, items.clone())return is unused.pub fn inject_program(module: &mut syn::ItemMod) { module.attrs.push(syn::parse_quote! { #[program] }); - module.content.as_mut().map(|(brace, items)| { - items.insert(0, syn::Item::Use(syn::parse_quote! { use super::*; })); - items.insert( - 1, - syn::Item::Struct(syn::parse_quote! { - #[derive(Accounts)] - pub struct VariadicBoltComponents<'info> { - pub authority: Signer<'info>, - } - }), - ); - (brace, items.clone()) - }); + if let Some((_brace, items)) = module.content.as_mut() { + items.insert(0, syn::Item::Use(syn::parse_quote! { use super::*; })); + items.insert( + 1, + syn::Item::Struct(syn::parse_quote! { + #[derive(Accounts)] + pub struct VariadicBoltComponents<'info> { + pub authority: anchor_lang::prelude::Signer<'info>, + } + }), + ); + } }crates/bolt-lang/src/instructions/update.rs (1)
47-61: Avoid cloning large account structs; pass references tovalidate.
session_token.clone()andauthority.clone()are unnecessary and can be costly.- let validity_ctx = crate::session_keys::ValidityChecker { - session_token: session_token.clone(), - session_signer: authority.clone(), + let validity_ctx = crate::session_keys::ValidityChecker { + session_token, + session_signer: authority, authority: component_authority, target_program: world::id_const(), }; - require!( - session_token.validate(validity_ctx)?, + require!( + session_token.validate(validity_ctx)?, crate::session_keys::SessionError::InvalidToken );examples/bundle/src/lib.rs (1)
37-44: Unify args parameter name (_args).Other examples use
_args. Keep naming consistent across systems.- pub fn execute(ctx: Context<Components>, _args_p: Vec<u8>) -> Result<Components> { + pub fn execute(ctx: Context<Components>, _args: Vec<u8>) -> Result<Components> {Also applies to: 55-60
crates/bolt-lang/attribute/src/system/mod.rs (5)
295-303: Avoid parse2(...).unwrap(); use parse_quote! to remove panicInline-parse with unwrap can panic during macro expansion. Use parse_quote!.
- let initialize_extra_accounts = quote! { - #[automatically_derived] - pub fn init_extra_accounts(_ctx: Context<#struct_name>) -> Result<()> { - Ok(()) - } - }; - content.push(syn::parse2(initialize_extra_accounts).unwrap()); + content.push(parse_quote! { + #[automatically_derived] + pub fn init_extra_accounts(_ctx: Context<#struct_name>) -> Result<()> { + Ok(()) + } + });
77-81: Don’t unwrap in panic path; show safe fallback nameUnwrapping in panic formatting can double-panic and hide context.
- panic!( - "Could not find the component bundle: {} in the module", - extractor.context_struct_name.unwrap() - ); + panic!( + "Could not find the component bundle: {} in the module", + extractor.context_struct_name.as_deref().unwrap_or("unknown") + );
90-95: Use meaningful fallback instead of unwrap_or_default()Avoid empty string on error messages; prefer a clear fallback.
- panic!( - "Could not find the component bundle: {} in the module", - extractor.context_struct_name.unwrap_or_default() - ); + panic!( + "Could not find the component bundle: {} in the module", + extractor.context_struct_name.as_deref().unwrap_or("unknown") + );
339-366: Defensive access to last path segment
.last_mut().unwrap()may panic on unusual TypePaths. Guard it.- let seg = tp.path.segments.last_mut().unwrap(); - match &mut seg.arguments { + if let Some(seg) = tp.path.segments.last_mut() { + match &mut seg.arguments { PathArguments::AngleBracketed(ab) => { if ab.args.is_empty() { ab.args.push(GenericArgument::Lifetime( syn::parse_quote!('info), )); } } - _ => { - seg.arguments = PathArguments::AngleBracketed( - syn::AngleBracketedGenericArguments { - colon2_token: None, - lt_token: Default::default(), - args: std::iter::once( - GenericArgument::Lifetime( - syn::parse_quote!('info), - ), - ) - .collect(), - gt_token: Default::default(), - }, - ); - } - } + _ => { + seg.arguments = PathArguments::AngleBracketed( + syn::AngleBracketedGenericArguments { + colon2_token: None, + lt_token: Default::default(), + args: std::iter::once( + GenericArgument::Lifetime( + syn::parse_quote!('info), + ), + ) + .collect(), + gt_token: Default::default(), + }, + ); + } + } + }
490-495: Avoid.expect(...)in Extractor; handle absence gracefullyPrefer a conditional check to prevent macro panics when shapes differ.
- let last_segment = type_path - .path - .segments - .last() - .expect("Context segment not found"); - if last_segment.ident == "Context" { + if let Some(last_segment) = type_path.path.segments.last() { + if last_segment.ident == "Context" { if let PathArguments::AngleBracketed(args) = &last_segment.arguments { for ga in args.args.iter() { if let syn::GenericArgument::Type(syn::Type::Path(type_path)) = ga { if let Some(first_seg) = type_path.path.segments.first() { self.context_struct_name = Some(first_seg.ident.to_string()); break; } } } } - } + } + }crates/programs/world/src/lib.rs (1)
284-291: Propagate real signer bit for authority in CPI accountsHardcoding
is_signer = falsewill break when caller signs; use the actual signer flag.- let accounts = vec![ - AccountMeta::new(ctx.accounts.payer.key(), true), - AccountMeta::new(ctx.accounts.data.key(), false), - AccountMeta::new_readonly(ctx.accounts.entity.key(), false), - AccountMeta::new_readonly(ctx.accounts.authority.key(), false), - AccountMeta::new_readonly(ctx.accounts.instruction_sysvar_account.key(), false), - AccountMeta::new_readonly(ctx.accounts.system_program.key(), false), - ]; + let accounts = vec![ + AccountMeta::new(ctx.accounts.payer.key(), true), + AccountMeta::new(ctx.accounts.data.key(), false), + AccountMeta::new_readonly(ctx.accounts.entity.key(), false), + AccountMeta::new_readonly( + ctx.accounts.authority.key(), + ctx.accounts.authority.is_signer, + ), + AccountMeta::new_readonly(ctx.accounts.instruction_sysvar_account.key(), false), + AccountMeta::new_readonly(ctx.accounts.system_program.key(), false), + ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockclients/typescript/src/generated/idl/world.jsonis excluded by!**/generated/**clients/typescript/src/generated/types/world.tsis excluded by!**/generated/**
📒 Files selected for processing (76)
.github/workflows/publish-bolt-crates.yml(1 hunks)Anchor.toml(2 hunks)Cargo.toml(3 hunks)clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs(2 hunks)clients/csharp/Solana.Unity.Bolt.Test/ECSTest.cs(5 hunks)clients/csharp/Solana.Unity.Bolt.Test/Framework.cs(2 hunks)clients/csharp/Solana.Unity.Bolt/ECS/Component.cs(1 hunks)clients/csharp/Solana.Unity.Bolt/ECS/Identifier.cs(1 hunks)clients/csharp/Solana.Unity.Bolt/ECS/System.cs(1 hunks)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt.cs(2 hunks)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/ApplySystem.cs(2 hunks)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.cs(1 hunks)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DestroyComponent.cs(1 hunks)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/InitializeComponent.cs(2 hunks)clients/csharp/Solana.Unity.Bolt/WorldProgram/Generated.cs(1 hunks)clients/csharp/Solana.Unity.Bolt/WorldProgram/World.cs(3 hunks)clients/typescript/src/delegation/delegate.ts(3 hunks)clients/typescript/src/ecs/component.ts(1 hunks)clients/typescript/src/ecs/identifier.ts(1 hunks)clients/typescript/src/ecs/index.ts(1 hunks)clients/typescript/src/ecs/system.ts(1 hunks)clients/typescript/src/index.ts(2 hunks)clients/typescript/src/world/transactions.ts(9 hunks)clients/typescript/test/framework.ts(2 hunks)clients/typescript/test/intermediate-level/acceleration.ts(3 hunks)clients/typescript/test/intermediate-level/ecs.ts(4 hunks)crates/bolt-cli/src/bundle.rs(1 hunks)crates/bolt-cli/src/lib.rs(5 hunks)crates/bolt-cli/src/rust_template.rs(2 hunks)crates/bolt-cli/src/templates/bundle/lib.rs.template(1 hunks)crates/bolt-cli/src/templates/bundle/mod.rs(1 hunks)crates/bolt-cli/src/templates/mod.rs(1 hunks)crates/bolt-cli/src/templates/workspace/workspace.toml.template(1 hunks)crates/bolt-lang/Cargo.toml(1 hunks)crates/bolt-lang/attribute/Cargo.toml(1 hunks)crates/bolt-lang/attribute/bolt-program/src/lib.rs(0 hunks)crates/bolt-lang/attribute/bundle/Cargo.toml(2 hunks)crates/bolt-lang/attribute/bundle/src/lib.rs(1 hunks)crates/bolt-lang/attribute/component-deserialize/Cargo.toml(1 hunks)crates/bolt-lang/attribute/component-deserialize/src/lib.rs(3 hunks)crates/bolt-lang/attribute/component/Cargo.toml(1 hunks)crates/bolt-lang/attribute/component/src/lib.rs(1 hunks)crates/bolt-lang/attribute/delegate/src/lib.rs(0 hunks)crates/bolt-lang/attribute/extra-accounts/src/lib.rs(1 hunks)crates/bolt-lang/attribute/src/bundle/mod.rs(1 hunks)crates/bolt-lang/attribute/src/common/mod.rs(1 hunks)crates/bolt-lang/attribute/src/component/attributes.rs(1 hunks)crates/bolt-lang/attribute/src/component/generate/mod.rs(1 hunks)crates/bolt-lang/attribute/src/component/generate/program.rs(1 hunks)crates/bolt-lang/attribute/src/component/mod.rs(1 hunks)crates/bolt-lang/attribute/src/delegate/mod.rs(1 hunks)crates/bolt-lang/attribute/src/lib.rs(1 hunks)crates/bolt-lang/attribute/src/system/mod.rs(1 hunks)crates/bolt-lang/attribute/system-input/src/lib.rs(2 hunks)crates/bolt-lang/attribute/system/Cargo.toml(1 hunks)crates/bolt-lang/attribute/system/src/lib.rs(1 hunks)crates/bolt-lang/src/cpi/mod.rs(1 hunks)crates/bolt-lang/src/errors.rs(1 hunks)crates/bolt-lang/src/instructions/destroy.rs(1 hunks)crates/bolt-lang/src/instructions/initialize.rs(1 hunks)crates/bolt-lang/src/instructions/mod.rs(1 hunks)crates/bolt-lang/src/instructions/update.rs(1 hunks)crates/bolt-lang/src/lib.rs(3 hunks)crates/bolt-lang/utils/src/lib.rs(2 hunks)crates/programs/bolt-component/Cargo.toml(0 hunks)crates/programs/bolt-component/src/lib.rs(0 hunks)crates/programs/bolt-system/Cargo.toml(0 hunks)crates/programs/bolt-system/Xargo.toml(0 hunks)crates/programs/bolt-system/src/lib.rs(0 hunks)crates/programs/world/Cargo.toml(1 hunks)crates/programs/world/src/lib.rs(6 hunks)crates/programs/world/src/utils.rs(1 hunks)docs/REPORT.md(1 hunks)examples/bundle/Cargo.toml(1 hunks)examples/bundle/src/lib.rs(1 hunks)scripts/test-publish.sh(1 hunks)
💤 Files with no reviewable changes (7)
- crates/programs/bolt-system/Xargo.toml
- crates/programs/bolt-component/Cargo.toml
- crates/programs/bolt-system/src/lib.rs
- crates/programs/bolt-system/Cargo.toml
- crates/bolt-lang/attribute/bolt-program/src/lib.rs
- crates/programs/bolt-component/src/lib.rs
- crates/bolt-lang/attribute/delegate/src/lib.rs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-15T16:28:50.695Z
Learnt from: notdanilo
PR: magicblock-labs/bolt#207
File: crates/bolt-lang/attribute/src/component/attributes.rs:35-56
Timestamp: 2025-10-15T16:28:50.695Z
Learning: In the Bolt Lang attribute system (crates/bolt-lang/attribute), component_id in the #[component] attribute is optional and can be empty by design. When not provided, it defaults to an empty string in the Attributes struct, with the code generator providing fallback logic (e.g., using the type name) at a later stage rather than during attribute parsing.
Applied to files:
crates/bolt-lang/attribute/component/src/lib.rscrates/bolt-lang/Cargo.tomlcrates/bolt-lang/attribute/src/component/generate/mod.rscrates/bolt-lang/attribute/src/component/attributes.rs
📚 Learning: 2025-10-15T15:51:36.753Z
Learnt from: notdanilo
PR: magicblock-labs/bolt#207
File: crates/programs/world/src/lib.rs:519-519
Timestamp: 2025-10-15T15:51:36.753Z
Learning: In the Bolt codebase, discriminator parameters should remain as `Vec<u8>` rather than `[u8; 8]` because the client code generation tooling (TypeScript/C#) cannot generate code from fixed-size array types.
Applied to files:
crates/bolt-lang/attribute/component-deserialize/src/lib.rsclients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.cscrates/programs/world/src/utils.rscrates/programs/world/src/lib.rs
🧬 Code graph analysis (44)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (2)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/ApplySystem.cs (4)
World(10-167)EntityType(143-166)EntityType(148-154)EntityType(156-165)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt.cs (2)
World(13-41)SerializeArgs(25-28)
crates/bolt-lang/attribute/component/src/lib.rs (1)
crates/bolt-lang/attribute/src/component/mod.rs (1)
process(13-31)
clients/csharp/Solana.Unity.Bolt/ECS/System.cs (3)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/ApplySystem.cs (4)
Solana(14-25)Solana(27-38)Solana(40-50)Solana(58-140)clients/typescript/src/ecs/system.ts (1)
System(4-12)clients/csharp/Solana.Unity.Bolt/ECS/Identifier.cs (2)
Identifier(8-38)Identifier(24-27)
clients/typescript/test/framework.ts (3)
clients/csharp/Solana.Unity.Bolt.Test/Framework.cs (2)
Framework(32-146)Framework(64-76)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/InitializeComponent.cs (1)
World(11-74)clients/csharp/Solana.Unity.Bolt/WorldProgram/World.cs (14)
PublicKey(36-46)PublicKey(48-55)PublicKey(57-66)PublicKey(68-77)PublicKey(79-91)PublicKey(93-96)PublicKey(98-105)PublicKey(106-111)PublicKey(113-119)PublicKey(121-132)PublicKey(136-143)PublicKey(145-152)PublicKey(188-191)PublicKey(192-200)
clients/typescript/src/delegation/delegate.ts (2)
clients/typescript/src/index.ts (1)
Component(20-20)clients/typescript/src/ecs/component.ts (1)
Component(5-27)
clients/csharp/Solana.Unity.Bolt/ECS/Component.cs (5)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/ApplySystem.cs (4)
Solana(14-25)Solana(27-38)Solana(40-50)Solana(58-140)clients/typescript/src/ecs/component.ts (1)
Component(5-27)clients/csharp/Solana.Unity.Bolt/ECS/Identifier.cs (2)
Identifier(8-38)Identifier(24-27)clients/csharp/Solana.Unity.Bolt/WorldProgram/World.cs (14)
PublicKey(36-46)PublicKey(48-55)PublicKey(57-66)PublicKey(68-77)PublicKey(79-91)PublicKey(93-96)PublicKey(98-105)PublicKey(106-111)PublicKey(113-119)PublicKey(121-132)PublicKey(136-143)PublicKey(145-152)PublicKey(188-191)PublicKey(192-200)clients/csharp/Solana.Unity.Bolt/ECS/System.cs (3)
System(9-26)System(15-15)System(20-25)
crates/bolt-cli/src/lib.rs (1)
crates/bolt-cli/src/bundle.rs (1)
new_bundle(7-42)
crates/bolt-lang/src/cpi/mod.rs (1)
clients/typescript/test/low-level/permissioning/world.ts (1)
world(4-234)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/ApplySystem.cs (6)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Generated.cs (17)
Solana(386-400)Solana(402-429)Solana(431-447)Solana(449-465)Solana(467-479)Solana(481-493)Solana(495-507)Solana(509-526)Solana(528-540)Solana(542-554)Solana(556-570)Solana(572-584)WorldProgram(383-585)ApplyWithSessionAccounts(284-294)World(75-122)World(90-121)ApplyAccounts(274-282)clients/csharp/Solana.Unity.Bolt/WorldProgram/World.cs (19)
Solana(24-28)Solana(30-34)Solana(254-279)Solana(281-307)PublicKey(36-46)PublicKey(48-55)PublicKey(57-66)PublicKey(68-77)PublicKey(79-91)PublicKey(93-96)PublicKey(98-105)PublicKey(106-111)PublicKey(113-119)PublicKey(121-132)PublicKey(136-143)PublicKey(145-152)PublicKey(188-191)PublicKey(192-200)WorldProgram(17-308)clients/csharp/Solana.Unity.Bolt/ECS/System.cs (3)
System(9-26)System(15-15)System(20-25)clients/csharp/Solana.Unity.Bolt/ECS/Component.cs (4)
Component(9-34)Component(15-15)Component(20-25)Seeds(30-33)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt.cs (2)
SerializeArgs(25-28)World(13-41)clients/csharp/Solana.Unity.Bolt/ECS/Identifier.cs (1)
GetMethodDiscriminator(33-37)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt.cs (2)
clients/typescript/src/index.ts (1)
GetDiscriminator(26-28)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/InitializeComponent.cs (1)
World(11-74)
crates/bolt-lang/src/instructions/initialize.rs (1)
crates/bolt-lang/src/cpi/mod.rs (1)
check(7-11)
crates/bolt-lang/attribute/src/bundle/mod.rs (5)
crates/bolt-lang/attribute/src/common/mod.rs (1)
generate_program(5-14)crates/bolt-lang/attribute/src/system/mod.rs (1)
transform_module_for_bundle(84-193)crates/bolt-lang/attribute/src/component/generate/program.rs (3)
generate_update(191-248)generate_instructions(13-23)remove_component_attributes(9-11)crates/bolt-lang/attribute/src/component/generate/mod.rs (2)
generate_implementation(17-24)enrich_type(9-15)crates/bolt-lang/attribute/src/delegate/mod.rs (1)
inject_delegate_items(6-36)
clients/typescript/test/intermediate-level/ecs.ts (3)
clients/typescript/src/world/transactions.ts (2)
InitializeComponent(397-442)ApplySystem(581-612)clients/typescript/src/ecs/component.ts (1)
Component(5-27)clients/typescript/src/ecs/system.ts (1)
System(4-12)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/InitializeComponent.cs (4)
clients/csharp/Solana.Unity.Bolt/WorldProgram/World.cs (15)
PublicKey(36-46)PublicKey(48-55)PublicKey(57-66)PublicKey(68-77)PublicKey(79-91)PublicKey(93-96)PublicKey(98-105)PublicKey(106-111)PublicKey(113-119)PublicKey(121-132)PublicKey(136-143)PublicKey(145-152)PublicKey(188-191)PublicKey(192-200)WorldProgram(17-308)clients/csharp/Solana.Unity.Bolt/WorldProgram/Generated.cs (4)
WorldProgram(383-585)InitializeComponentAccounts(325-339)Entity(27-49)Entity(34-48)clients/csharp/Solana.Unity.Bolt/ECS/Component.cs (4)
Component(9-34)Component(15-15)Component(20-25)Seeds(30-33)clients/csharp/Solana.Unity.Bolt/ECS/Identifier.cs (1)
GetMethodDiscriminator(33-37)
crates/bolt-lang/src/instructions/update.rs (1)
crates/bolt-lang/src/cpi/mod.rs (1)
check(7-11)
clients/typescript/src/ecs/component.ts (2)
clients/typescript/src/index.ts (2)
Component(20-20)FindComponentPda(109-122)clients/typescript/src/ecs/identifier.ts (1)
Identifier(4-18)
crates/bolt-cli/src/templates/mod.rs (1)
crates/bolt-lang/attribute/bundle/src/lib.rs (1)
bundle(9-11)
clients/typescript/test/intermediate-level/acceleration.ts (3)
clients/typescript/src/world/transactions.ts (1)
InitializeComponent(397-442)clients/typescript/src/ecs/component.ts (1)
Component(5-27)clients/typescript/src/delegation/delegate.ts (1)
DelegateComponent(154-199)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.cs (4)
clients/typescript/src/delegation/delegate.ts (1)
DelegateComponent(154-199)clients/csharp/Solana.Unity.Bolt/WorldProgram/World.cs (15)
PublicKey(36-46)PublicKey(48-55)PublicKey(57-66)PublicKey(68-77)PublicKey(79-91)PublicKey(93-96)PublicKey(98-105)PublicKey(106-111)PublicKey(113-119)PublicKey(121-132)PublicKey(136-143)PublicKey(145-152)PublicKey(188-191)PublicKey(192-200)WorldProgram(17-308)clients/typescript/src/index.ts (1)
FindComponentPda(109-122)clients/csharp/Solana.Unity.Bolt/ECS/Component.cs (4)
Component(9-34)Component(15-15)Component(20-25)Seeds(30-33)
crates/bolt-cli/src/bundle.rs (2)
crates/bolt-cli/src/rust_template.rs (1)
create_bundle(32-45)crates/bolt-cli/src/workspace.rs (1)
with_workspace(9-24)
clients/typescript/src/ecs/identifier.ts (2)
clients/typescript/src/index.ts (1)
GetDiscriminator(26-28)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt.cs (1)
GetDiscriminator(30-40)
clients/csharp/Solana.Unity.Bolt.Test/Framework.cs (1)
clients/csharp/Solana.Unity.Bolt/WorldProgram/World.cs (14)
PublicKey(36-46)PublicKey(48-55)PublicKey(57-66)PublicKey(68-77)PublicKey(79-91)PublicKey(93-96)PublicKey(98-105)PublicKey(106-111)PublicKey(113-119)PublicKey(121-132)PublicKey(136-143)PublicKey(145-152)PublicKey(188-191)PublicKey(192-200)
crates/bolt-lang/attribute/bundle/src/lib.rs (3)
crates/bolt-lang/attribute/src/system/mod.rs (1)
process(35-82)crates/bolt-lang/attribute/src/bundle/mod.rs (1)
process(10-80)crates/bolt-lang/attribute/src/component/mod.rs (1)
process(13-31)
examples/bundle/src/lib.rs (4)
crates/bolt-lang/attribute/bundle/src/lib.rs (1)
bundle(9-11)crates/bolt-lang/attribute/component/src/lib.rs (1)
component(17-19)crates/bolt-lang/attribute/system/src/lib.rs (1)
system(4-6)crates/bolt-lang/attribute/system-input/src/lib.rs (1)
system_input(20-133)
crates/bolt-lang/attribute/src/component/generate/program.rs (4)
crates/bolt-lang/src/lib.rs (3)
seed(61-61)size(62-62)try_from_account_info(89-97)crates/bolt-lang/src/instructions/destroy.rs (1)
destroy(5-39)crates/bolt-lang/src/instructions/initialize.rs (1)
initialize(4-14)crates/bolt-lang/src/instructions/update.rs (2)
update(6-31)update_with_session(33-78)
clients/typescript/src/ecs/system.ts (2)
clients/csharp/Solana.Unity.Bolt/ECS/System.cs (3)
System(9-26)System(15-15)System(20-25)clients/typescript/src/ecs/identifier.ts (1)
Identifier(4-18)
clients/typescript/src/index.ts (1)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt.cs (1)
GetDiscriminator(30-40)
crates/bolt-lang/attribute/src/component/mod.rs (5)
crates/bolt-lang/attribute/src/common/mod.rs (1)
generate_program(5-14)crates/bolt-lang/attribute/src/delegate/mod.rs (2)
inject_delegate_items(6-36)vec(22-32)crates/bolt-lang/attribute/src/component/attributes.rs (3)
from(14-20)from(24-32)from(36-56)crates/bolt-lang/attribute/src/component/generate/mod.rs (2)
generate_implementation(17-24)enrich_type(9-15)crates/bolt-lang/attribute/src/component/generate/program.rs (4)
generate_instructions(13-23)vec(36-41)vec(236-244)generate_update(191-248)
crates/bolt-lang/attribute/src/component/generate/mod.rs (2)
crates/bolt-lang/utils/src/lib.rs (1)
add_bolt_metadata(4-22)crates/bolt-lang/src/lib.rs (2)
seed(61-61)size(62-62)
crates/bolt-lang/src/instructions/destroy.rs (1)
crates/bolt-lang/src/cpi/mod.rs (1)
check(7-11)
crates/bolt-lang/attribute/src/lib.rs (3)
crates/bolt-lang/attribute/bundle/src/lib.rs (1)
bundle(9-11)crates/bolt-lang/attribute/component/src/lib.rs (1)
component(17-19)crates/bolt-lang/attribute/system/src/lib.rs (1)
system(4-6)
crates/bolt-lang/attribute/system/src/lib.rs (1)
crates/bolt-lang/attribute/src/system/mod.rs (1)
process(35-82)
crates/bolt-cli/src/rust_template.rs (2)
crates/bolt-cli/src/templates/bundle/mod.rs (1)
create_bundle_template(5-16)crates/bolt-cli/src/templates/workspace/mod.rs (3)
workspace_manifest(4-6)cargo_toml_with_serde(47-55)xargo_toml(57-59)
crates/bolt-lang/attribute/src/delegate/mod.rs (3)
crates/programs/world/src/lib.rs (6)
seed(735-737)seed(794-796)seed(814-816)seed(824-826)pda(743-745)pda(802-804)crates/bolt-lang/src/lib.rs (2)
seed(61-61)discriminator_from_account_info(99-104)crates/bolt-lang/attribute/src/component/generate/program.rs (2)
vec(36-41)vec(236-244)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DestroyComponent.cs (4)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Generated.cs (4)
WorldProgram(383-585)DestroyComponentAccounts(307-323)Entity(27-49)Entity(34-48)clients/csharp/Solana.Unity.Bolt/WorldProgram/World.cs (15)
WorldProgram(17-308)PublicKey(36-46)PublicKey(48-55)PublicKey(57-66)PublicKey(68-77)PublicKey(79-91)PublicKey(93-96)PublicKey(98-105)PublicKey(106-111)PublicKey(113-119)PublicKey(121-132)PublicKey(136-143)PublicKey(145-152)PublicKey(188-191)PublicKey(192-200)clients/typescript/src/world/transactions.ts (1)
DestroyComponent(343-385)clients/csharp/Solana.Unity.Bolt/ECS/Component.cs (4)
Component(9-34)Component(15-15)Component(20-25)Seeds(30-33)
crates/bolt-lang/src/lib.rs (1)
crates/bolt-lang/attribute/bundle/src/lib.rs (1)
bundle(9-11)
crates/bolt-lang/src/instructions/mod.rs (3)
crates/bolt-lang/src/instructions/destroy.rs (1)
destroy(5-39)crates/bolt-lang/src/instructions/initialize.rs (1)
initialize(4-14)crates/bolt-lang/src/instructions/update.rs (1)
update(6-31)
crates/bolt-lang/attribute/src/system/mod.rs (2)
crates/bolt-lang/attribute/src/bundle/mod.rs (1)
process(10-80)crates/bolt-lang/src/lib.rs (1)
parse_args(50-55)
crates/bolt-lang/attribute/src/component/attributes.rs (1)
crates/bolt-lang/attribute/component-id/src/lib.rs (1)
component_id(9-15)
clients/csharp/Solana.Unity.Bolt/ECS/Identifier.cs (3)
clients/typescript/src/index.ts (2)
Program(18-18)GetDiscriminator(26-28)clients/typescript/src/ecs/identifier.ts (1)
Identifier(4-18)clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt.cs (2)
World(13-41)GetDiscriminator(30-40)
clients/typescript/src/world/transactions.ts (3)
clients/typescript/src/index.ts (5)
Component(20-20)Program(18-18)FindComponentProgramDataPda(97-106)System(20-20)SerializeArgs(129-138)clients/typescript/src/ecs/component.ts (2)
Component(5-27)pda(16-22)clients/typescript/src/ecs/system.ts (1)
System(4-12)
clients/csharp/Solana.Unity.Bolt/WorldProgram/World.cs (2)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/ApplySystem.cs (5)
Solana(14-25)Solana(27-38)Solana(40-50)Solana(58-140)World(10-167)clients/csharp/Solana.Unity.Bolt/WorldProgram/Generated.cs (16)
Solana(386-400)Solana(402-429)Solana(431-447)Solana(449-465)Solana(467-479)Solana(481-493)Solana(495-507)Solana(509-526)Solana(528-540)Solana(542-554)Solana(556-570)Solana(572-584)ApplyAccounts(274-282)World(75-122)World(90-121)ApplyWithSessionAccounts(284-294)
crates/programs/world/src/lib.rs (2)
crates/programs/world/src/utils.rs (1)
discriminator_for(19-36)clients/typescript/src/world/transactions.ts (2)
InitializeComponent(397-442)DestroyComponent(343-385)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Generated.cs (3)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/InitializeComponent.cs (6)
World(11-74)Task(17-20)Task(22-25)Task(27-30)Task(32-45)Task(56-73)clients/csharp/Solana.Unity.Bolt/ECS/Component.cs (3)
Component(9-34)Component(15-15)Component(20-25)clients/typescript/src/world/transactions.ts (1)
InitializeComponent(397-442)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: setup
- GitHub Check: setup
| var instruction = Bolt.World.ApplySystem( | ||
| framework.WorldPda, | ||
| framework.SystemSimpleMovement, | ||
| new Bolt.World.EntityType[] { | ||
| new Bolt.World.EntityType(framework.AccelerationEntityPda, | ||
| new PublicKey[] { framework.ExampleComponentPosition }) | ||
| }, | ||
| Bolt.World.SerializeArgs(new { direction = "Up" }), | ||
| framework.Wallet.Account.PublicKey | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Prefer the object-args overload; avoid manual serialization.
Use the ApplySystem overload that accepts object args to remove explicit SerializeArgs and reduce call-site errors.
- var instruction = Bolt.World.ApplySystem(
+ var instruction = Bolt.World.ApplySystem(
framework.WorldPda,
framework.SystemSimpleMovement,
new Bolt.World.EntityType[] {
new Bolt.World.EntityType(framework.AccelerationEntityPda,
new PublicKey[] { framework.ExampleComponentPosition })
},
- Bolt.World.SerializeArgs(new { direction = "Up" }),
+ new { direction = "Up" },
framework.Wallet.Account.PublicKey
);If you’re migrating this path to bundled systems soon, consider the System-based overload to auto-handle discriminators and remaining accounts.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var instruction = Bolt.World.ApplySystem( | |
| framework.WorldPda, | |
| framework.SystemSimpleMovement, | |
| new Bolt.World.EntityType[] { | |
| new Bolt.World.EntityType(framework.AccelerationEntityPda, | |
| new PublicKey[] { framework.ExampleComponentPosition }) | |
| }, | |
| Bolt.World.SerializeArgs(new { direction = "Up" }), | |
| framework.Wallet.Account.PublicKey | |
| ); | |
| var instruction = Bolt.World.ApplySystem( | |
| framework.WorldPda, | |
| framework.SystemSimpleMovement, | |
| new Bolt.World.EntityType[] { | |
| new Bolt.World.EntityType( | |
| framework.AccelerationEntityPda, | |
| new PublicKey[] { framework.ExampleComponentPosition } | |
| ) | |
| }, | |
| new { direction = "Up" }, | |
| framework.Wallet.Account.PublicKey | |
| ); |
🤖 Prompt for AI Agents
In clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs around lines 50 to
59, the call to Bolt.World.ApplySystem manually serializes arguments with
Bolt.World.SerializeArgs(new { direction = "Up" }); change this to the
ApplySystem overload that accepts object args directly (pass new { direction =
"Up" } without SerializeArgs) so serialization is handled by the API and reduces
call-site errors; if you plan to migrate to bundled systems soon, consider using
the System-based overload which will auto-handle discriminators and remaining
accounts.
| ExampleComponentPosition = new PublicKey(Position.Program.PositionProgram.ID); | ||
| ExampleComponentVelocity = new PublicKey(Velocity.Program.VelocityProgram.ID); | ||
| SystemSimpleMovement = new PublicKey("FSa6qoJXFBR3a7ThQkTAMrC15p6NkchPEjBdd4n6dXxA"); | ||
| ExampleBundleProgramId = new PublicKey("CgfPBUeDUL3GT6b5AUDFE56KKgU4ycWA9ERjEWsfMZCj"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Avoid hard‑coding the bundle program id; read from env/config.
Literal program ids cause flaky tests across clusters. Prefer ENV with fallback.
Apply:
- ExampleBundleProgramId = new PublicKey("CgfPBUeDUL3GT6b5AUDFE56KKgU4ycWA9ERjEWsfMZCj");
+ var bundlePid = Environment.GetEnvironmentVariable("EXAMPLE_BUNDLE_PROGRAM_ID");
+ ExampleBundleProgramId = new PublicKey(
+ string.IsNullOrWhiteSpace(bundlePid)
+ ? "CgfPBUeDUL3GT6b5AUDFE56KKgU4ycWA9ERjEWsfMZCj"
+ : bundlePid
+ );📝 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.
| ExampleBundleProgramId = new PublicKey("CgfPBUeDUL3GT6b5AUDFE56KKgU4ycWA9ERjEWsfMZCj"); | |
| var bundlePid = Environment.GetEnvironmentVariable("EXAMPLE_BUNDLE_PROGRAM_ID"); | |
| ExampleBundleProgramId = new PublicKey( | |
| string.IsNullOrWhiteSpace(bundlePid) | |
| ? "CgfPBUeDUL3GT6b5AUDFE56KKgU4ycWA9ERjEWsfMZCj" | |
| : bundlePid | |
| ); |
🤖 Prompt for AI Agents
In clients/csharp/Solana.Unity.Bolt.Test/Framework.cs around line 75, the bundle
program id is hard‑coded which causes flaky tests across clusters; change it to
read from an environment variable or test config with a fallback to the current
literal. Use Environment.GetEnvironmentVariable (or a test config provider) to
retrieve a string like BUNDLE_PROGRAM_ID, validate/parse it into a PublicKey,
and only if not present or invalid fall back to the existing
"CgfPBUeDUL3GT6b5AUDFE56KKgU4ycWA9ERjEWsfMZCj"; ensure you log or surface an
error if parsing fails so tests don’t silently run with an incorrect id.
| /// <summary> | ||
| /// Build seeds: (seed ?? "") + (Name ?? ""), mirroring TS seeds(). | ||
| /// </summary> | ||
| public string Seeds(string seed = null) | ||
| { | ||
| return (seed ?? "") + (Name ?? ""); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Seed prefix order is inverted; breaks bundled PDAs. Also add 32‑byte length guard.
Spec and TS require name + seed. Current code returns seed + name, deriving different PDAs than on-chain.
Apply this diff and guard against oversized seeds:
- /// Build seeds: (seed ?? "") + (Name ?? ""), mirroring TS seeds().
+ /// Build seeds: (Name ?? "") + (seed ?? ""), mirroring TS seeds() and bundle spec.
public string Seeds(string seed = null)
{
- return (seed ?? "") + (Name ?? "");
+ var s = (Name ?? "") + (seed ?? "");
+ // Each PDA seed must be <= 32 bytes; combined string is one seed.
+ if (System.Text.Encoding.UTF8.GetByteCount(s) > 32)
+ throw new ArgumentException("Component seed (name + seed) exceeds 32 bytes.");
+ return s;
}Run to spot call sites that might assume old order:
🏁 Script executed:
#!/bin/bash
rg -n --type cs -C2 '\.Seeds\('Length of output: 3260
Flip seed order and enforce 32-byte guard in Seeds
- Change Seeds to
(Name ?? "") + (seed ?? "")and throw if UTF-8 byte length >32:
- /// Build seeds: (seed ?? "") + (Name ?? ""), mirroring TS seeds().
+ /// Build seeds: (Name ?? "") + (seed ?? ""), mirroring TS seeds() and bundle spec.
public string Seeds(string seed = null)
{
- return (seed ?? "") + (Name ?? "");
+ var s = (Name ?? "") + (seed ?? "");
+ if (System.Text.Encoding.UTF8.GetByteCount(s) > 32)
+ throw new ArgumentException("Component seed (name + seed) exceeds 32 bytes.");
+ return s;
}- Verify these call sites to ensure new order and guard satisfy on-chain PDAs:
• WorldProgram/Bolt/InitializeComponent.cs:60
• WorldProgram/Bolt/DestroyComponent.cs:51
• WorldProgram/Bolt/DelegateComponent.cs:64
• WorldProgram/Bolt/ApplySystem.cs:80
📝 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.
| /// <summary> | |
| /// Build seeds: (seed ?? "") + (Name ?? ""), mirroring TS seeds(). | |
| /// </summary> | |
| public string Seeds(string seed = null) | |
| { | |
| return (seed ?? "") + (Name ?? ""); | |
| } | |
| /// <summary> | |
| /// Build seeds: (Name ?? "") + (seed ?? ""), mirroring TS seeds() and bundle spec. | |
| /// </summary> | |
| public string Seeds(string seed = null) | |
| { | |
| var s = (Name ?? "") + (seed ?? ""); | |
| if (System.Text.Encoding.UTF8.GetByteCount(s) > 32) | |
| throw new ArgumentException("Component seed (name + seed) exceeds 32 bytes."); | |
| return s; | |
| } |
🤖 Prompt for AI Agents
In clients/csharp/Solana.Unity.Bolt/ECS/Component.cs around lines 27 to 33,
change Seeds to build the seed as (Name ?? "") + (seed ?? "") instead of the
current order, and add a guard that computes the UTF-8 byte length of the
resulting string and throws an exception if it exceeds 32 bytes; update the
method to throw a clear ArgumentException when the byte length > 32. After
implementing this change, review and adjust call sites to match the new
order/guard at WorldProgram/Bolt/InitializeComponent.cs:60,
WorldProgram/Bolt/DestroyComponent.cs:51,
WorldProgram/Bolt/DelegateComponent.cs:64, and
WorldProgram/Bolt/ApplySystem.cs:80 to ensure on-chain PDA derivation remains
correct.
| @@ -0,0 +1,7 @@ | |||
| extern crate proc_macro; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Unnecessary extern crate in Rust 2018+
extern crate proc_macro; is redundant in 2018+ edition for a regular library crate. Remove to reduce lint noise.
-extern crate proc_macro;
+📝 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.
| extern crate proc_macro; | |
| // (the `extern crate proc_macro;` declaration at the top of this file has been removed) |
🤖 Prompt for AI Agents
In crates/bolt-lang/attribute/src/lib.rs around line 1, the file contains an
unnecessary `extern crate proc_macro;` which is redundant under Rust 2018+;
remove that line so the crate uses the proc_macro items via the prelude (or by
importing specific items with `use proc_macro::...` where needed) to eliminate
lint noise.
| [dependencies] | ||
| syn = { workspace = true, features = ["visit-mut"] } | ||
| quote = { workspace = true } | ||
| proc-macro2 = { workspace = true } | ||
| proc-macro2 = { workspace = true } | ||
| bolt-attribute.workspace = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Workspace dep shorthand is fine; ensure workspace provides it and no cycles.
proc-macro2 = { workspace = true }andbolt-attribute.workspace = truelook good if the root has[workspace.dependencies] proc-macro2andbolt-attributedefined.- Please verify
bolt-attributeis declared in the workspace and that adding it here doesn’t create a proc‑macro dependency cycle.
🏁 Script executed:
#!/bin/bash
# Check that workspace declares these dependencies
rg -nP '(?m)^\[workspace\.dependencies\][\s\S]*\b(proc-macro2|bolt-attribute)\b' -C2 Cargo.toml
# Quick heuristic for potential cycle: does bolt-attribute depend back on this crate?
rg -nP '(?m)^\[dependencies\][\s\S]*\bbolt-attribute-bolt-system\b' -C2 -S -g 'crates/**/Cargo.toml'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Locate any workspace.dependencies sections
rg -n 'workspace.dependencies' -g 'Cargo.toml'
# Show context around root workspace.dependencies
rg -nP '(?m)^\[workspace\.dependencies\]' -C5 -g 'Cargo.toml'
# Print this crate's Cargo.toml
sed -n '1,50p' crates/bolt-lang/attribute/system/Cargo.tomlLength of output: 1212
Workspace shorthand requires root Cargo.toml entries for external crates
- Add
proc-macro2,syn, andquoteunder[workspace.dependencies]in the rootCargo.tomlto back{ workspace = true }usage. bolt-attributeis already declared at line 22; no proc-macro dependency cycle detected.
🤖 Prompt for AI Agents
In crates/bolt-lang/attribute/system/Cargo.toml around lines 14 to 18, the crate
uses workspace shorthand ({ workspace = true }) for syn, quote and proc-macro2
but the root Cargo.toml does not declare them under [workspace.dependencies];
open the root Cargo.toml and add entries for proc-macro2, syn and quote under
[workspace.dependencies] (matching the versions used elsewhere in the workspace
or the versions required by these crates) so the workspace = true references
resolve correctly, then run cargo metadata/build to verify no missing dependency
errors.
crates/bolt-lang/src/cpi/mod.rs
Outdated
| let instruction = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative(0, &instruction_sysvar_account.to_account_info()).map_err(|_| BoltError::InvalidCaller)?; | ||
| require_eq!(instruction.program_id, World::id(), BoltError::InvalidCaller); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Micro-alloc: avoid cloning AccountInfo.
You can pass instruction_sysvar_account directly to get_instruction_relative and skip to_account_info() cloning.
- let instruction = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative(0, &instruction_sysvar_account.to_account_info()).map_err(|_| BoltError::InvalidCaller)?;
+ let instruction = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative(
+ 0,
+ instruction_sysvar_account,
+ ).map_err(|_| BoltError::InvalidCaller)?;📝 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 instruction = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative(0, &instruction_sysvar_account.to_account_info()).map_err(|_| BoltError::InvalidCaller)?; | |
| require_eq!(instruction.program_id, World::id(), BoltError::InvalidCaller); | |
| let instruction = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative( | |
| 0, | |
| instruction_sysvar_account, | |
| ).map_err(|_| BoltError::InvalidCaller)?; | |
| require_eq!(instruction.program_id, World::id(), BoltError::InvalidCaller); |
🤖 Prompt for AI Agents
In crates/bolt-lang/src/cpi/mod.rs around lines 8-9, avoid cloning the
AccountInfo by removing the to_account_info() call; pass the existing
instruction_sysvar_account directly (by reference) into
anchor_lang::solana_program::sysvar::instructions::get_instruction_relative so
the function receives the AccountInfo without creating a clone and adjust the
borrow if needed (e.g., pass instruction_sysvar_account or
&instruction_sysvar_account depending on its type).
| /// Returned if the account mismatch | ||
| #[msg("Account mismatch")] | ||
| AccountMismatch, | ||
| /// Component is not delegateable | ||
| #[msg("Component is not delegateable")] | ||
| ComponentNotDelegateable, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
New error variants ok; mind on-chain error numbering stability.
Appending is safe; avoid reordering existing variants and update client mappings if any rely on numeric codes.
🏁 Script executed:
#!/bin/bash
# Check clients reference these variants by name/number
rg -nP 'AccountMismatch|ComponentNotDelegateable' clients/ -C2Length of output: 2577
| pub const fn discriminator_for(name: &str) -> [u8; 8] { | ||
| let mut discriminator = [0u8; 8]; | ||
|
|
||
| let hash = const_crypto::sha2::Sha256::new() | ||
| .update(name.as_bytes()) | ||
| .finalize(); | ||
|
|
||
| let hash_bytes = hash.as_slice(); | ||
|
|
||
| // Manual loop required for const fn compatibility | ||
| let mut i = 0; | ||
| while i < 8 { | ||
| discriminator[i] = hash_bytes[i]; | ||
| i += 1; | ||
| } | ||
|
|
||
| discriminator | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Good const discriminator; add Vec wrapper for client-facing APIs.
Per learnings, clients can’t handle [u8; 8] in generated code. Provide a non-const wrapper returning Vec for instruction params.
Based on learnings
pub const fn discriminator_for(name: &str) -> [u8; 8] {
...
discriminator
}
+
+/// Convenience: returns discriminator as Vec<u8> for client/API use.
+#[inline]
+pub fn discriminator_for_vec(name: &str) -> Vec<u8> {
+ discriminator_for(name).to_vec()
+}📝 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 const fn discriminator_for(name: &str) -> [u8; 8] { | |
| let mut discriminator = [0u8; 8]; | |
| let hash = const_crypto::sha2::Sha256::new() | |
| .update(name.as_bytes()) | |
| .finalize(); | |
| let hash_bytes = hash.as_slice(); | |
| // Manual loop required for const fn compatibility | |
| let mut i = 0; | |
| while i < 8 { | |
| discriminator[i] = hash_bytes[i]; | |
| i += 1; | |
| } | |
| discriminator | |
| } | |
| pub const fn discriminator_for(name: &str) -> [u8; 8] { | |
| let mut discriminator = [0u8; 8]; | |
| let hash = const_crypto::sha2::Sha256::new() | |
| .update(name.as_bytes()) | |
| .finalize(); | |
| let hash_bytes = hash.as_slice(); | |
| // Manual loop required for const fn compatibility | |
| let mut i = 0; | |
| while i < 8 { | |
| discriminator[i] = hash_bytes[i]; | |
| i += 1; | |
| } | |
| discriminator | |
| } | |
| /// Convenience: returns discriminator as Vec<u8> for client/API use. | |
| #[inline] | |
| pub fn discriminator_for_vec(name: &str) -> Vec<u8> { | |
| discriminator_for(name).to_vec() | |
| } |
🤖 Prompt for AI Agents
In crates/programs/world/src/utils.rs around lines 19 to 36, the const fn
discriminator_for returns a [u8; 8], which client code generators struggle to
consume; add a non-const public wrapper function (e.g., pub fn
discriminator_for_vec(name: &str) -> Vec<u8>) that calls the existing const fn
(or duplicates its logic non-const) and returns the 8-byte discriminator as a
Vec<u8> so generated client APIs receive a Vec<u8> instead of an array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DestroyComponent.cs (1)
46-65: Critical: Missing discriminator for bundled component destroy and misleading documentation.This overload has two issues:
Missing custom discriminator for bundled components: Unlike
InitializeComponentwhich callscomponent.GetMethodDiscriminator("initialize")and usesInitializeComponentWithDiscriminator, this code calls the plainWorldProgram.DestroyComponent()which uses a hardcoded discriminator. Bundled components require component-specific discriminators for destroy operations (as shown in Rust and TypeScript implementations). You must either:
- Generate
DestroyComponentWithDiscriminatormethod inWorldProgram.Generated.cs(if not already present), or- Call
component.GetMethodDiscriminator("destroy")and use the appropriate discriminator variant.Documentation inconsistency (line 47): XML docs state "seed defaults to component name" but the parameter default is
seed = ""(empty string). The actual behavior usescomponent.Seeds(seed)which concatenates the provided seed with the component name. Update docs to clarify the seed concatenation behavior.
♻️ Duplicate comments (62)
docs/REPORT.md (1)
7-8: Add series labels for chart clarity (duplicate of previous review comment).While the performance data has been updated, the two bar series still lack labels. This makes the chart legend ambiguous and prevents readers from understanding which series represents which deployment approach.
As noted in the previous review, add explicit series labels (e.g., "Bundle" and "Standalone") matching the bar ordering:
- bar [13110,20640,28678,37169,46306,55754,65697,76307,87375,98746] - bar [4581,8633,13163,18120,23526,29382,35706,42669,49889,57556] + bar "Bundle" [13110,20640,28678,37169,46306,55754,65697,76307,87375,98746] + bar "Standalone" [4581,8633,13163,18120,23526,29382,35706,42669,49889,57556]Optional: Consider setting
y-axisto start at 0 for clearer visual comparison between series.crates/bolt-lang/src/errors.rs (1)
11-16: Update client mappings for new error variant.The
ComponentNotDelegateablevariant is not yet reflected in generated client mappings (TypeScript IDL, C# types). Regenerate the IDL and rerun client codegen to ensure the new error code is available to client code.AccountMismatchis already mapped, so maintain existing error numbering.crates/bolt-cli/src/templates/workspace/workspace.toml.template (1)
3-6: Future‑proof the members glob (optional).The workspace members list explicitly enumerates three ECS subdirectories. Collapsing these into a single two-level glob
"programs-ecs/*/*"would automatically include any future first-level subdirectories without requiring template updates.Apply this diff:
members = [ "programs/*", - "programs-ecs/components/*", - "programs-ecs/systems/*", - "programs-ecs/bundles/*" + "programs-ecs/*/*" ]clients/typescript/src/index.ts (1)
5-5: Node-only crypto import breaks browser compatibility.The direct import of Node's
cryptomodule will fail in browser environments. If browser support is in scope, provide a platform-agnostic implementation using Web Crypto API (crypto.subtle.digest) as a fallback.Also applies to: 26-28
crates/bolt-lang/src/cpi/mod.rs (1)
8-12: Avoid unnecessary AccountInfo clone.The
to_account_info()call creates an unnecessary clone.get_instruction_relativeaccepts&AccountInfodirectly, so you can passinstruction_sysvar_accountwithout cloning.Apply this diff:
let instruction = anchor_lang::solana_program::sysvar::instructions::get_instruction_relative( 0, - &instruction_sysvar_account.to_account_info(), + instruction_sysvar_account, ) .map_err(|_| BoltError::InvalidCaller)?;examples/bundle/Cargo.toml (2)
3-3: Use workspace inheritance for version.For consistency with other example crates, the version should use workspace inheritance rather than a hardcoded value.
Apply this diff:
-version = "0.2.5" +version = { workspace = true }
25-25: Consider workspace inheritance for serde dependency.For consistency with other dependencies, consider using workspace inheritance for
serdeif it's defined in the workspace manifest.Apply this diff:
-serde = { version = "1.0", features = ["derive"] } +serde = { workspace = true, features = ["derive"] }crates/bolt-lang/attribute/component-deserialize/src/lib.rs (2)
4-4: Missing Cargo dependency for sha2This is a previously flagged critical issue. The
use sha2::{Digest, Sha256};import requires addingsha2tocrates/bolt-lang/attribute/component-deserialize/Cargo.toml.
42-44: Discriminator prefix mismatch across languagesThis is a previously flagged critical issue. Rust uses
"account:{name}"as the hash input, but TypeScript'sGetDiscriminator(name)(clients/typescript/src/index.ts:25-27) and C#'s implementation hash the rawnamestring without the prefix. This will produce different discriminators and break cross-language compatibility.clients/csharp/Solana.Unity.Bolt/ECS/System.cs (1)
9-26: Previously flagged issues remainMultiple refactor suggestions from previous reviews apply to this class:
- Class name
Systemcollides with BCL namespace- Constructor parameter
nameshould be nullable with default nullFrom(object)method should have typed overloads to avoid boxingclients/typescript/src/ecs/identifier.ts (2)
2-2: Circular import dependencyThis is a previously flagged issue. Importing
GetDiscriminatorfrom../indexcreates a circular dependency (index.ts → ecs/index.ts → ecs/component.ts → ecs/identifier.ts → index.ts). ExtractGetDiscriminatorto a standalone utility module.
5-11: Fields should be readonlyPreviously flagged: The
programandnamefields should be markedreadonlyto prevent accidental mutation.Cargo.toml (1)
35-35: Missing component-large examplePreviously flagged critical issue: The
component-largedependency referencespath = "examples/component-large"which may not exist.crates/bolt-lang/attribute/bundle/Cargo.toml (1)
2-3: Uninformative package descriptionPreviously flagged style issue: The description simply repeats the package name. Consider a more descriptive description like "Bolt bundle attribute for combining components and systems".
crates/bolt-lang/attribute/extra-accounts/src/lib.rs (1)
53-66: Fully qualify Anchor types and errors in macro output.Avoid collisions by returning anchor_lang::Result and qualifying AccountInfo, ErrorCode, and Context.
[ suggest_essential_refactor ]
- fn #field_name(&self) -> Result<&'c AccountInfo<'info>>; + fn #field_name(&self) -> anchor_lang::Result<&'c anchor_lang::prelude::AccountInfo<'info>>; @@ - fn #field_name(&self) -> Result<&'c AccountInfo<'info>> { - self.remaining_accounts.get(<T as bolt_lang::NumberOfComponents>::NUMBER_OF_COMPONENTS + #index).ok_or_else(|| ErrorCode::ConstraintAccountIsNone.into()) + fn #field_name(&self) -> anchor_lang::Result<&'c anchor_lang::prelude::AccountInfo<'info>> { + self.remaining_accounts + .get(<T as bolt_lang::NumberOfComponents>::NUMBER_OF_COMPONENTS + #index) + .ok_or_else(|| anchor_lang::error::ErrorCode::ConstraintAccountIsNone.into()) } @@ - impl<'a, 'b, 'c, 'info, T: bolt_lang::Bumps + bolt_lang::NumberOfComponents> ContextExtensions<'a, 'b, 'c, 'info, T> for Context<'a, 'b, 'c, 'info, T> { + impl<'a, 'b, 'c, 'info, T: bolt_lang::Bumps + bolt_lang::NumberOfComponents> + ContextExtensions<'a, 'b, 'c, 'info, T> + for anchor_lang::prelude::Context<'a, 'b, 'c, 'info, T> + { #(#helper_functions_impl)* }Also applies to: 73-82
Anchor.toml (1)
13-13: Use snake_case program key to match crate/IDL name.Change example-bundle → example_bundle so Anchor resolves anchor.workspace.ExampleBundle correctly.
-example-bundle = "CgfPBUeDUL3GT6b5AUDFE56KKgU4ycWA9ERjEWsfMZCj" +example_bundle = "CgfPBUeDUL3GT6b5AUDFE56KKgU4ycWA9ERjEWsfMZCj"crates/bolt-lang/attribute/src/lib.rs (1)
1-1: Remove redundant extern crate (Rust 2018+).Not needed; reduces lint noise.
-extern crate proc_macro; +clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
50-59: Prefer the object-args overload to avoid manual serialization.The
ApplySystemAPI provides an overload that acceptsobject argsdirectly. Remove the explicitSerializeArgscall to simplify this code.Apply this diff:
var instruction = Bolt.World.ApplySystem( framework.WorldPda, framework.SystemSimpleMovement, new Bolt.World.EntityType[] { new Bolt.World.EntityType(framework.AccelerationEntityPda, new PublicKey[] { framework.ExampleComponentPosition }) }, - Bolt.World.SerializeArgs(new { direction = "Up" }), + new { direction = "Up" }, framework.Wallet.Account.PublicKey );clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt.cs (1)
30-40: UseSHA256.HashDataand validate input.Replace the disposable
SHA256.Create()with the staticSHA256.HashDatamethod for better performance, and add validation to guard against null or empty names.Apply this diff:
public static byte[] GetDiscriminator(string name) { // Anchor uses the first 8 bytes of the SHA256 hash of the instruction name. // See: https://github.com/coral-xyz/anchor/blob/master/lang/syn/src/codegen/accounts/discriminator.rs - var nameBytes = Encoding.UTF8.GetBytes(name); - using (var sha256 = SHA256.Create()) { - var hash = sha256.ComputeHash(nameBytes); - var discriminator = new byte[8]; - Array.Copy(hash, discriminator, 8); - return discriminator; - } + if (string.IsNullOrEmpty(name)) throw new ArgumentException("name must be non-empty", nameof(name)); + var hash = SHA256.HashData(Encoding.UTF8.GetBytes(name)); + var discriminator = new byte[8]; + Array.Copy(hash, discriminator, 8); + return discriminator; }clients/typescript/src/ecs/system.ts (1)
9-11: HardenSystem.fromagainst cross-bundle instances.The
instanceofcheck can fail when multiple bundles load the System class. Consider acceptingPublicKeyInitData(string/Buffer/Uint8Array) and using duck-typing instead.Apply this diff:
- static from(systemId: PublicKey | System): System { - return systemId instanceof System ? systemId : new System(systemId); - } + static from(systemId: System | PublicKey | string | Uint8Array): System { + if (systemId instanceof System) return systemId; + const pk = systemId instanceof PublicKey ? systemId : new PublicKey(systemId as any); + return new System(pk); + }clients/typescript/src/delegation/delegate.ts (1)
179-181: UseconstforownerProgram.The
ownerProgramvariable is never reassigned; declare it withconstfor clarity.Apply this diff:
const component = Component.from(componentId); -let ownerProgram = component.program; +const ownerProgram = component.program; const componentPda = component.pda(entity, seed);clients/csharp/Solana.Unity.Bolt/WorldProgram/Generated.cs (1)
509-526: Guard discriminator length to prevent buffer issues.While callers currently validate the discriminator, adding an internal guard prevents misuse and improves robustness.
Apply this diff:
public static Solana.Unity.Rpc.Models.TransactionInstruction InitializeComponentWithDiscriminator(InitializeComponentAccounts accounts, byte[] discriminator, PublicKey programId = null) { byte[] IX_INITIALIZE_COMPONENT_WITH_DISCRIMINATOR = new byte[] { 174, 196, 222, 15, 149, 54, 137, 23 }; programId ??= new(ID); + if (discriminator == null || discriminator.Length != 8) + throw new ArgumentException("discriminator must be 8 bytes", nameof(discriminator)); List<Solana.Unity.Rpc.Models.AccountMeta> keys = new()crates/bolt-cli/src/templates/bundle/mod.rs (1)
5-16: LGTM! Optional: Considerimpl AsRef<Path>for ergonomics.The template generation logic is correct. For improved ergonomics, you could accept
impl AsRef<Path>to allow bothPathandPathBufarguments directly.Optional diff:
-pub fn create_bundle_template(name: &str, program_path: &Path) -> Files { +pub fn create_bundle_template(name: &str, program_path: impl AsRef<Path>) -> Files { let program_id = anchor_cli::rust_template::get_or_create_program_id(name); let program_name = name.to_snake_case(); vec![( - program_path.join("src").join("lib.rs"), + program_path.as_ref().join("src").join("lib.rs"), format!(crates/bolt-lang/attribute/bundle/src/lib.rs (1)
5-7: Update documentation to reflect support for multiple components and systems.The current wording "one
#[component]and one#[system]" is misleading since the processor handles multiple items of each type.Apply this diff:
-/// Combines one `#[component]` and one `#[system]` into a single Anchor `#[program]` module. +/// Combines one or more `#[component]` and `#[system]` items into a single Anchor `#[program]` module.clients/csharp/Solana.Unity.Bolt.Test/Framework.cs (1)
75-75: Consider using environment variable for bundle program ID.Hard-coded program IDs can cause test failures across different clusters or deployments. Reading from an environment variable with a fallback improves test robustness.
Apply this diff:
- ExampleBundleProgramId = new PublicKey("CgfPBUeDUL3GT6b5AUDFE56KKgU4ycWA9ERjEWsfMZCj"); + var bundlePid = Environment.GetEnvironmentVariable("EXAMPLE_BUNDLE_PROGRAM_ID"); + ExampleBundleProgramId = new PublicKey( + string.IsNullOrWhiteSpace(bundlePid) + ? "CgfPBUeDUL3GT6b5AUDFE56KKgU4ycWA9ERjEWsfMZCj" + : bundlePid + );crates/bolt-cli/src/bundle.rs (3)
6-6: Fix incorrect comment.The comment says "component" but this function creates a bundle.
Apply this diff:
-// Create a new component from the template +// Create a new bundle from the template
10-12: Return an error instead of printing and continuing.Currently when the workspace parent is missing, the function prints a message but doesn't fail. This can lead to silent failures and unexpected behavior.
Apply this diff:
- None => { - println!("Unable to make new bundle"); - } + None => { + return Err(anyhow!("Unable to make new bundle: workspace path has no parent")); + }
34-35: Write Anchor.toml to the config path instead of CWD.Relying on
set_current_dirand writing to "Anchor.toml" is fragile. Use the config's path to write the file directly.Apply this diff:
let toml = cfg.to_string(); - fs::write("Anchor.toml", toml)?; + fs::write(cfg.path(), toml)?;crates/bolt-lang/attribute/component/Cargo.toml (1)
15-15: Verify proc-macro deps; add syn/quote/proc-macro2 if referenced.If this crate uses
syn,quote, orproc_macro2directly, compilation will fail without explicit deps.Run:
#!/bin/bash # Check for direct references in this crate rg -nP --type=rust -C2 '\b(syn|quote|proc_macro2)::' crates/bolt-lang/attribute/component || trueIf matches exist, add:
[dependencies] bolt-attribute.workspace = true +proc-macro2.workspace = true +quote.workspace = true +syn.workspace = trueclients/csharp/Solana.Unity.Bolt/ECS/Identifier.cs (2)
12-17: Make properties immutable (private set or init-only).Avoid external mutation of identity fields.
- public PublicKey Program { get; set; } + public PublicKey Program { get; private set; } @@ - public string Name { get; set; } + public string? Name { get; private set; }
24-27: Constructor should accept optional/nullable name.Current callers pass null; align the API and suppress NRT warnings.
- public Identifier(PublicKey program, string name) { + public Identifier(PublicKey program, string? name = null) { this.Program = program; this.Name = name; }crates/bolt-lang/src/instructions/initialize.rs (1)
1-1: Remove unnecessary bounds and unused import.
Account<'info, T>already enforces Anchor (de)serialize + Clone; this function only needsT: Default. Also drop the unusedBorshDeserializeimport.- use crate::borsh::BorshDeserialize; use anchor_lang::prelude::*; @@ -pub fn initialize< - 'info, - T: Default + AccountSerialize + AccountDeserialize + BorshDeserialize + Clone, ->( +pub fn initialize<'info, T: Default>(Also applies to: 4-7
clients/typescript/test/intermediate-level/acceleration.ts (1)
58-67: Use correct framework field for bundled position PDA.Replace
componentPositionEntity1PdawithbundlePositionEntity1Pda.- framework.componentPositionEntity1Pda = + framework.bundlePositionEntity1Pda = createAcceleratedBundledComponentPosition.componentPda; @@ - const position = await framework.exampleBundle.account.position.fetch( - framework.componentPositionEntity1Pda, - ); + const position = await framework.exampleBundle.account.position.fetch( + framework.bundlePositionEntity1Pda, + );clients/typescript/src/ecs/component.ts (1)
24-26: Guard PDA seed length (32 bytes).Avoid opaque PDA derivation failures by checking byte length.
- seeds(seed?: string): string { - return (this.name ?? "") + (seed ?? ""); - } + seeds(seed?: string): string { + const s = (this.name ?? "") + (seed ?? ""); + if (Buffer.byteLength(s, "utf8") > 32) { + throw new Error("Component seed (name + seed) exceeds 32 bytes."); + } + return s; + }clients/csharp/Solana.Unity.Bolt/ECS/Component.cs (1)
27-33: Fix seed order (Name + seed) and enforce 32‑byte limit.The C# implementation has the seed order reversed compared to TypeScript. Verified that
clients/typescript/src/ecs/component.tsuses(this.name ?? "") + (seed ?? ""), but the C# version currently does(seed ?? "") + (Name ?? ""). This causes PDAs to be derived incorrectly across all four call sites (DestroyComponent, ApplySystem, DelegateComponent, InitializeComponent), breaking compatibility with TypeScript and on-chain behavior.- /// Build seeds: (seed ?? "") + (Name ?? ""), mirroring TS seeds(). + /// Build seeds: (Name ?? "") + (seed ?? ""), mirroring TS seeds(). public string Seeds(string seed = null) { - return (seed ?? "") + (Name ?? ""); + var s = (Name ?? "") + (seed ?? ""); + if (System.Text.Encoding.UTF8.GetByteCount(s) > 32) + throw new global::System.ArgumentException("Component seed (name + seed) exceeds 32 bytes."); + return s; }clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/InitializeComponent.cs (1)
47-73: Critical: Validate component.Name and fix Seeds ordering.Two issues must be addressed:
Missing Name validation: When
component.Nameis null or empty,GetMethodDiscriminatorwill generate the global "initialize" discriminator instead of the component-specific one (e.g., "position_initialize"). This will cause bundled components to fail. Add validation:if (string.IsNullOrEmpty(component.Name)) throw new ArgumentException("Bundled component requires a non-empty Name.", nameof(component));Seeds ordering mismatch: The
Component.Seedsmethod inECS/Component.csreturns(seed ?? "") + (Name ?? ""), but TypeScript returns(this.name ?? "") + (seed ?? ""). This causes PDA derivation to produce different addresses! FixComponent.Seedsto match TypeScript:public string Seeds(string seed = null) { return (Name ?? "") + (seed ?? ""); }Based on past review comments, these issues were previously identified but not yet resolved.
clients/csharp/Solana.Unity.Bolt/WorldProgram/Bolt/DelegateComponent.cs (2)
22-57: Critical issues: Hard-coded discriminator and endianness.Two problems in this overload:
Hard-coded discriminator at line 31: The discriminator
{ 90, 147, 75, 178, 85, 88, 4, 137 }is hard-coded. For bundled components, this should be derived usingcomponent.GetMethodDiscriminator("delegate")to ensure correct routing.Platform-dependent endianness at line 33:
BitConverter.GetBytes(commitFrequencyMs)uses the system's native endianness. Solana requires little-endian. Apply:-byte[] commitFrequencyBytes = BitConverter.GetBytes(commitFrequencyMs); +byte[] commitFrequencyBytes = BitConverter.GetBytes(commitFrequencyMs); +if (!BitConverter.IsLittleEndian) Array.Reverse(commitFrequencyBytes);
63-95: Critical: Add validation and fix discriminator/endianness.This bundled component overload has three issues:
Missing parameter validation: Add checks consistent with
InitializeComponent:if (component is null) throw new ArgumentNullException(nameof(component)); var discriminator = component.GetMethodDiscriminator("delegate"); if (discriminator is null || discriminator.Length != 8) throw new ArgumentException("Invalid discriminator", nameof(component));Hard-coded discriminator at line 69: Replace the hard-coded byte array with the validated discriminator from step 1.
Platform-dependent endianness at line 71: Same issue as the other overload - ensure little-endian encoding.
clients/csharp/Solana.Unity.Bolt.Test/ECSTest.cs (1)
111-129: Use structured deserialization like Position.Replace manual byte parsing with the generated Velocity deserializer for consistency and safety.
- var data = Convert.FromBase64String(accountInfo.Data[0]); - int offset = 8; // skip discriminator - long x = BitConverter.ToInt64(data, offset); offset += 8; - long y = BitConverter.ToInt64(data, offset); offset += 8; - long z = BitConverter.ToInt64(data, offset); offset += 8; - Debug.Assert(1 == x, "X is not equal to 1"); - Debug.Assert(2 == y, "Y is not equal to 2"); - Debug.Assert(3 == z, "Z is not equal to 3"); + var data = Convert.FromBase64String(accountInfo.Data[0]); + var velocity = Velocity.Accounts.Velocity.Deserialize(data); + Debug.Assert(1 == velocity.X, "X is not equal to 1"); + Debug.Assert(2 == velocity.Y, "Y is not equal to 2"); + Debug.Assert(3 == velocity.Z, "Z is not equal to 3");crates/bolt-lang/attribute/src/common/mod.rs (1)
18-30: Remove unnecessary items.clone() and map().Mutate in place; avoid cloning the entire items vec.
- module.content.as_mut().map(|(brace, items)| { - items.insert(0, syn::Item::Use(syn::parse_quote! { use super::*; })); - items.insert( - 1, - syn::Item::Struct(syn::parse_quote! { - #[derive(Accounts)] - pub struct VariadicBoltComponents<'info> { - pub authority: Signer<'info>, - } - }), - ); - (brace, items.clone()) - }); + if let Some((_, items)) = module.content.as_mut() { + items.insert(0, syn::Item::Use(syn::parse_quote! { use super::*; })); + items.insert(1, syn::Item::Struct(syn::parse_quote! { + #[derive(Accounts)] + pub struct VariadicBoltComponents<'info> { + pub authority: Signer<'info>, + } + })); + }crates/bolt-lang/attribute/src/delegate/mod.rs (7)
55-61: Verify discriminator helper and account field name.Line 56 calls
BoltMetadata::discriminator_from_account_infobut the root-leveldiscriminator_from_account_infofunction (see relevant snippets) is the correct helper. Also verify the account field name matches the struct definition at line 95.Check the discriminator helper signature and ensure consistency:
#!/bin/bash # Verify the correct discriminator helper function rg -n "pub fn discriminator_from_account_info" crates/bolt-lang/src/ # Verify DelegateInput struct field names ast-grep --pattern $'pub struct DelegateInput { $$$ }'
20-35: Avoid map for side effects and remove unnecessary clone.The
.map()closure mutatesitemsthen returns(brace, items.clone()), but the return value is discarded and the clone is wasteful. Useif letfor in-place mutation.Apply this diff:
- module.content.as_mut().map(|(brace, items)| { + if let Some((_, items)) = module.content.as_mut() { items.extend( vec![ delegate_fn, delegate_struct, reinit_undelegate_fn, reinit_undelegate_struct, undelegate_fn, undelegate_struct, ] .into_iter() .map(|item| syn::parse2(item).expect("Failed to parse delegate item")) .collect::<Vec<_>>(), ); - (brace, items.clone()) - }); + }
49-51: Critical: PDA seed mismatch between delegate and component initialization.Line 50 constructs seeds as
[component::seed(), name.as_bytes(), entity.key()], but bundled components use[name.as_bytes(), entity.key()](see program.rs:102). This mismatch will cause delegation to fail.Apply this diff:
let component_matches = components.iter().map(|(component, name)| quote! { - #component::DISCRIMINATOR => &[#component::seed(), #name.as_bytes(), &ctx.accounts.entity.key().to_bytes()] + #component::DISCRIMINATOR => &[#name.as_bytes(), &ctx.accounts.entity.key().to_bytes()] }).collect::<Vec<_>>();
96-96: Fix documentation typos in CHECK comments.Multiple CHECK comments have stray trailing backticks. They should be
/// CHECK:without trailing punctuation.Remove the trailing backticks on lines 96, 101, 104, 107, 109, 110:
- /// CHECK:` + /// CHECK:Also applies to: 101-101, 104-104, 107-107, 109-110
114-133: Use crate::ID for robustness.Line 125 uses
id()which may be unavailable in nested modules. Usecrate::IDinstead for clarity and robustness.Apply this diff:
::bolt_lang::undelegate_account( delegated_account, - &id(), + &crate::ID, buffer, payer, system_program, account_seeds, )?;
139-139: Fix CHECK typos and verify payer should be Signer.Lines 139, 142, 145, 148 have stray backticks in CHECK comments. Additionally, line 147 declares
payerasAccountInfo<'info>, but if it funds the undelegation (debits lamports), it should beSigner<'info>.Fix CHECK typos and consider:
- /// CHECK:` + /// CHECK: #[account(mut)] - pub payer: AccountInfo<'info>, + pub payer: Signer<'info>,Also applies to: 142-142, 145-148
153-182: Fully qualify MagicProgram type.Line 180 uses unqualified
MagicProgramwhich may not be in scope in the generated code. Use the fully qualified path::bolt_lang::MagicProgram.Apply this diff:
- pub magic_program: Program<'info, MagicProgram> + pub magic_program: Program<'info, ::bolt_lang::MagicProgram>crates/bolt-lang/attribute/src/component/generate/program.rs (4)
34-57: Critical: Apply struct modifications are discarded.The
.map()closure computesmodified_itemsbut never assigns it back toitems. Line 44 clones items, so modifications tostruct_itemin line 48 affect only the clones. The result of.map()(line 34) is discarded, leaving the original module unchanged.Apply this diff to fix:
- module.content.as_mut().map(|(brace, items)| { + if let Some((_, items)) = module.content.as_mut() { items.extend( vec![initialize_fn, initialize_struct, destroy_fn, destroy_struct] .into_iter() - .map(|item| { - syn::parse2(item).expect("Failed to parse generate initialize and destroy item") - }) + .map(|item| syn::parse2(item).expect("Failed to parse generated instruction item")) .collect::<Vec<_>>(), ); - let modified_items: Vec<syn::Item> = items - .iter_mut() - .map(|item| match item.clone() { - syn::Item::Struct(mut struct_item) - if struct_item.ident == "Apply" || struct_item.ident == "ApplyWithSession" => - { - modify_apply_struct(&mut struct_item); - syn::Item::Struct(struct_item) - } - _ => item.clone(), - }) - .collect(); - (brace, modified_items) - }); + for item in items.iter_mut() { + if let syn::Item::Struct(struct_item) = item { + if struct_item.ident == "Apply" || struct_item.ident == "ApplyWithSession" { + modify_apply_struct(struct_item); + } + } + } + }
99-105: Extract duplicated PDA seed generation logic.Lines 99-105 and 153-159 contain identical PDA seed generation logic. Extracting this into a helper function improves maintainability.
Consider this helper:
fn generate_pda_seeds(component_type: &Type, component_name: Option<&String>) -> TokenStream2 { if let Some(name) = component_name { let name_bytes = LitByteStr::new(name.as_bytes(), component_type.span()); quote! { [#name_bytes, entity.key().as_ref()] } } else { quote! { [<#component_type>::seed(), entity.key().as_ref()] } } }Then use:
let seeds_tokens = generate_pda_seeds(component_type, component_name);Also applies to: 153-159
250-253: String-based type checking may cause false positives.Using
.contains("Program")matches any type with "Program" in the name (e.g.,ProgramData,MyProgramConfig). Use proper type path analysis instead.Apply this diff:
fn is_expecting_program(field: &Field) -> bool { - field.ty.to_token_stream().to_string().contains("Program") + if let Type::Path(type_path) = &field.ty { + type_path.path.segments.first().map_or(false, |seg| seg.ident == "Program") + } else { + false + } }
234-247: Remove unnecessary clone and discarded return value.After extending
itemsin place (line 235), the code returns(brace, items.clone())from a.map()whose result is discarded. The clone is wasteful.Apply this diff:
- module.content.as_mut().map(|(brace, items)| { + if let Some((_, items)) = module.content.as_mut() { items.extend( vec![ update_fn, update_struct, update_with_session_fn, update_with_session_struct, ] .into_iter() .map(|item| syn::parse2(item).expect("Failed to parse generate update item")) .collect::<Vec<_>>(), ); - (brace, items.clone()) - }); + }crates/bolt-lang/attribute/src/component/attributes.rs (1)
18-19: Improve error diagnostics in proc macro parsing.Using
.unwrap()in proc macro code causes unhelpful compiler errors. Replace with.expect()for clearer diagnostics.Apply this diff:
- .map(|attr| Self::from(attr.parse_meta().unwrap())) + .map(|attr| Self::from(attr.parse_meta().expect("Invalid #[component] attribute meta")))examples/bundle/src/lib.rs (1)
37-37: Standardize parameter naming for consistency.Parameter naming
_args_pis inconsistent with the convention used in other examples (typically_args). Consider standardizing for consistency.Apply this diff:
- pub fn execute(ctx: Context<Components>, _args_p: Vec<u8>) -> Result<Components> { + pub fn execute(ctx: Context<Components>, _args: Vec<u8>) -> Result<Components> {Apply similarly at line 55.
Also applies to: 55-55
crates/bolt-lang/src/instructions/update.rs (1)
47-49: Consider avoiding clones if ValidityChecker accepts references.Cloning
session_tokenandauthoritymay be expensive. IfValidityCheckercan accept references instead of owned values, consider passing references to improve performance.crates/bolt-lang/attribute/src/component/generate/mod.rs (2)
26-47: Fallback to type name when component_id is empty.When
component_idis an empty string (which is allowed by design per the attribute parser), the generatedseed()function will return an empty byte slice, causing PDA derivation issues. The generator should fallback to the type name.Based on learnings
Apply this diff:
let name = &input.ident; - let component_id_value = &attributes.component_id; + let seed_value = if attributes.component_id.is_empty() { + name.to_string() + } else { + attributes.component_id.clone() + }; let implementation = quote! { #[automatically_derived] impl ComponentTraits for #name { fn seed() -> &'static [u8] { - #component_id_value.as_bytes() + #seed_value.as_bytes() } fn size() -> usize { 8 + <#name>::INIT_SPACE } } };
49-90: Critical: Init struct includes bolt_metadata causing duplicate field.The
Initstruct (lines 57-60) includes all fields includingbolt_metadata, but line 80 explicitly assignsbolt_metadata: BoltMetadata::default(). This creates a duplicate field error in the generated code.Apply this diff:
// Generate fields for the init struct - let init_struct_fields = fields.named.iter().map(|f| { - let name = &f.ident; - let ty = &f.ty; - quote! { pub #name: #ty } - }); + let init_struct_fields = fields.named.iter().filter_map(|f| { + let name = f.ident.as_ref()?; + if name == "bolt_metadata" { return None; } + let ty = &f.ty; + Some(quote! { pub #name: #ty }) + }); // Generate struct initialization code using the init struct - let struct_init_fields = fields.named.iter().map(|f| { - let name = &f.ident; - quote! { #name: init_struct.#name } - }); + let struct_init_fields = fields.named.iter().filter_map(|f| { + let name = f.ident.as_ref()?; + if name == "bolt_metadata" { return None; } + Some(quote! { #name: init_struct.#name }) + });crates/bolt-lang/attribute/src/system/mod.rs (5)
77-81: Improve panic error message to avoid nested panic.If
context_struct_nameisNone, theunwrap()on line 79 will panic during error message formatting, producing a confusing error.Apply this diff:
} else { panic!( "Could not find the component bundle: {} in the module", - extractor.context_struct_name.unwrap() + extractor.context_struct_name.unwrap_or_else(|| "unknown".to_string()) ); }
90-95: Use meaningful fallback in panic message.Using
unwrap_or_default()produces an empty/unhelpful string whencontext_struct_nameisNone. Use a descriptive fallback instead.Apply this diff:
if extractor.field_count.is_none() { panic!( "Could not find the component bundle: {} in the module", - extractor.context_struct_name.unwrap_or_default() + extractor.context_struct_name.as_deref().unwrap_or("unknown") ); }
105-143: Consider extracting inline visitor structs to module level.The inline
SystemRenameandExecRenamevisitor structs make this function harder to read and test. Consider extracting them as module-level private structs.This would improve code organization and testability by moving the visitor implementations to module scope while keeping the same functionality.
302-302: Replace unwrap with expect for better error diagnostics.Using
.unwrap()when parsing generated code produces unhelpful errors. Use.expect()with a descriptive message.Apply this diff:
- content.push(syn::parse2(initialize_extra_accounts).unwrap()); + content.push(syn::parse2(initialize_extra_accounts).expect("Failed to parse init_extra_accounts"));
337-341: Use safe access for path segments.Line 340 calls
.unwrap()onlast_mut()which could panic if the path has no segments. Use a safe access pattern.Consider:
Type::Path(mut tp) => { - let seg = tp.path.segments.last_mut().unwrap(); + if let Some(seg) = tp.path.segments.last_mut() { match &mut seg.arguments { PathArguments::AngleBracketed(ab) => { if ab.args.is_empty() { ab.args.push(GenericArgument::Lifetime( syn::parse_quote!('info), )); } } _ => { seg.arguments = PathArguments::AngleBracketed( syn::AngleBracketedGenericArguments { colon2_token: None, lt_token: Default::default(), args: std::iter::once( GenericArgument::Lifetime( syn::parse_quote!('info), ), ) .collect(), gt_token: Default::default(), }, ); } } + } Type::Path(tp) }crates/programs/world/src/lib.rs (1)
287-294:⚠️ Critical: Propagate authority signer flag in CPI.Line 291 hardcodes
is_signer = falsefor the authority account. Since the check at line 280 allows authority to be either a signer OR the World program ID, the downstream component program needs the actual signer status. When a signing authority invokes this function, the component program will receive the account as non-signer, potentially causing authorization failures.Apply this diff to propagate the real signer bit:
let accounts = vec![ AccountMeta::new(ctx.accounts.payer.key(), true), AccountMeta::new(ctx.accounts.data.key(), false), AccountMeta::new_readonly(ctx.accounts.entity.key(), false), - AccountMeta::new_readonly(ctx.accounts.authority.key(), false), + AccountMeta::new_readonly( + ctx.accounts.authority.key(), + ctx.accounts.authority.is_signer, + ), AccountMeta::new_readonly(ctx.accounts.instruction_sysvar_account.key(), false), AccountMeta::new_readonly(ctx.accounts.system_program.key(), false), ];
| public static async Task ApplySimpleMovementSystemUpOnEntity1(Framework framework) { | ||
| var apply = new ApplyAccounts() { | ||
| Authority = framework.Wallet.Account.PublicKey, | ||
| BoltSystem = framework.SystemSimpleMovement, | ||
| World = framework.WorldPda, | ||
| }; | ||
| var instruction = WorldProgram.Apply(apply, Bolt.World.SerializeArgs(new { direction = "Up" })); | ||
| instruction.Keys.Add(AccountMeta.ReadOnly(framework.ExampleComponentPosition, false)); | ||
| instruction.Keys.Add(AccountMeta.Writable(framework.ComponentPositionEntity1Pda, false)); | ||
| var instruction = Bolt.World.ApplySystem( | ||
| framework.WorldPda, | ||
| framework.SystemSimpleMovement, | ||
| new Bolt.World.EntityType[] { | ||
| new Bolt.World.EntityType(framework.Entity1Pda, | ||
| new PublicKey[] { framework.ExampleComponentPosition }) | ||
| }, | ||
| Bolt.World.SerializeArgs(new { direction = "Up" }), | ||
| framework.Wallet.Account.PublicKey | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Prefer a single ApplySystem overload across tests.
Use the object-args overload (like in Right) for readability and parity.
- Bolt.World.SerializeArgs(new { direction = "Up" }),
+ new { direction = "Up" },📝 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.
| public static async Task ApplySimpleMovementSystemUpOnEntity1(Framework framework) { | |
| var apply = new ApplyAccounts() { | |
| Authority = framework.Wallet.Account.PublicKey, | |
| BoltSystem = framework.SystemSimpleMovement, | |
| World = framework.WorldPda, | |
| }; | |
| var instruction = WorldProgram.Apply(apply, Bolt.World.SerializeArgs(new { direction = "Up" })); | |
| instruction.Keys.Add(AccountMeta.ReadOnly(framework.ExampleComponentPosition, false)); | |
| instruction.Keys.Add(AccountMeta.Writable(framework.ComponentPositionEntity1Pda, false)); | |
| var instruction = Bolt.World.ApplySystem( | |
| framework.WorldPda, | |
| framework.SystemSimpleMovement, | |
| new Bolt.World.EntityType[] { | |
| new Bolt.World.EntityType(framework.Entity1Pda, | |
| new PublicKey[] { framework.ExampleComponentPosition }) | |
| }, | |
| Bolt.World.SerializeArgs(new { direction = "Up" }), | |
| framework.Wallet.Account.PublicKey | |
| ); | |
| public static async Task ApplySimpleMovementSystemUpOnEntity1(Framework framework) { | |
| var instruction = Bolt.World.ApplySystem( | |
| framework.WorldPda, | |
| framework.SystemSimpleMovement, | |
| new Bolt.World.EntityType[] { | |
| new Bolt.World.EntityType(framework.Entity1Pda, | |
| new PublicKey[] { framework.ExampleComponentPosition }) | |
| }, | |
| new { direction = "Up" }, | |
| framework.Wallet.Account.PublicKey | |
| ); |
🤖 Prompt for AI Agents
In clients/csharp/Solana.Unity.Bolt.Test/ECSTest.cs around lines 158-168, the
test uses the positional ApplySystem overload; replace it with the single
object-args ApplySystem overload (the same form used in the "Right" test) for
consistency and readability. Construct a single anonymous object containing the
entities list (one entry referencing framework.Entity1Pda and its component
public key), the args object with direction = "Up", and the signer/public key,
and pass framework.WorldPda and framework.SystemSimpleMovement as the first two
parameters as in the other tests; this keeps the call shape consistent across
tests.
| public byte[] GetMethodDiscriminator(string method) | ||
| { | ||
| var prefix = string.IsNullOrEmpty(Name) ? "" : Name + "_"; | ||
| return World.GetDiscriminator("global:" + prefix + method); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Validate method argument.
Guard null/empty to prevent accidental bad discriminators.
- public byte[] GetMethodDiscriminator(string method)
- {
- var prefix = string.IsNullOrEmpty(Name) ? "" : Name + "_";
- return World.GetDiscriminator("global:" + prefix + method);
- }
+ public byte[] GetMethodDiscriminator(string method)
+ {
+ if (string.IsNullOrWhiteSpace(method))
+ throw new global::System.ArgumentException("method cannot be null or whitespace", nameof(method));
+ var prefix = string.IsNullOrEmpty(Name) ? "" : Name + "_";
+ return World.GetDiscriminator("global:" + prefix + method);
+ }Ensure Component/System ctors also accept nullable names after this change.
🤖 Prompt for AI Agents
In clients/csharp/Solana.Unity.Bolt/ECS/Identifier.cs around lines 33 to 37,
validate the method parameter before building the discriminator: throw an
ArgumentException (or ArgumentNullException) when method is null or empty, and
use the validated value when calling World.GetDiscriminator; update the logic
that builds the prefix to remain unchanged for Name but only proceed when method
is valid. Also update Component and System constructors to accept nullable names
(adjust signatures/types and internal handling) so they tolerate a null Name
while preserving existing behavior for other fields.
| public static Solana.Unity.Rpc.Models.TransactionInstruction ApplyWithDiscriminator(ApplyAccounts accounts, byte[] system_discriminator, byte[] args, PublicKey programId = null) | ||
| { | ||
| programId ??= new(ID); | ||
| List<Solana.Unity.Rpc.Models.AccountMeta> keys = new() | ||
| { | ||
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.BoltSystem, false), | ||
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.Authority, true), | ||
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.InstructionSysvarAccount, false), | ||
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.World, false) | ||
| }; | ||
| byte[] _data = new byte[1200]; | ||
| int offset = 0; | ||
| _data.WriteSpan(IX_APPLY_WITH_DISCRIMINATOR, offset); | ||
| offset += 8; | ||
| _data.WriteS32(system_discriminator.Length, offset); | ||
| offset += 4; | ||
| _data.WriteSpan(system_discriminator, offset); | ||
| offset += system_discriminator.Length; | ||
| _data.WriteS32(args.Length, offset); | ||
| offset += 4; | ||
| _data.WriteSpan(args, offset); | ||
| offset += args.Length; | ||
| byte[] resultData = new byte[offset]; | ||
| Array.Copy(_data, resultData, offset); | ||
| return new Solana.Unity.Rpc.Models.TransactionInstruction{Keys = keys, ProgramId = programId.KeyBytes, Data = resultData}; | ||
| } | ||
|
|
||
| public static Solana.Unity.Rpc.Models.TransactionInstruction ApplyWithSessionAndDiscriminator(ApplyWithSessionAccounts accounts, byte[] system_discriminator, byte[] args, PublicKey programId = null) | ||
| { | ||
| programId ??= new(ID); | ||
| List<Solana.Unity.Rpc.Models.AccountMeta> keys = new() | ||
| { | ||
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.BoltSystem, false), | ||
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.Authority, true), | ||
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.InstructionSysvarAccount, false), | ||
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.World, false), | ||
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.SessionToken, false) | ||
| }; | ||
| byte[] _data = new byte[1200]; | ||
| int offset = 0; | ||
| _data.WriteSpan(IX_APPLY_WITH_SESSION_AND_DISCRIMINATOR, offset); | ||
| offset += 8; | ||
| _data.WriteS32(system_discriminator.Length, offset); | ||
| offset += 4; | ||
| _data.WriteSpan(system_discriminator, offset); | ||
| offset += system_discriminator.Length; | ||
| _data.WriteS32(args.Length, offset); | ||
| offset += 4; | ||
| _data.WriteSpan(args, offset); | ||
| offset += args.Length; | ||
| byte[] resultData = new byte[offset]; | ||
| Array.Copy(_data, resultData, offset); | ||
| return new Solana.Unity.Rpc.Models.TransactionInstruction{Keys = keys, ProgramId = programId.KeyBytes, Data = resultData}; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Right-size buffers and validate discriminator length.
Avoid fixed 1200B allocation and reject non-8B discriminators early.
- byte[] _data = new byte[1200];
+ if (system_discriminator == null || system_discriminator.Length != 8)
+ throw new ArgumentException("system_discriminator must be 8 bytes", nameof(system_discriminator));
+ int size = IX_APPLY_WITH_DISCRIMINATOR.Length + 4 + system_discriminator.Length + 4 + args.Length;
+ byte[] _data = new byte[size];
@@
- byte[] resultData = new byte[offset];
+ byte[] resultData = new byte[offset];
Array.Copy(_data, resultData, offset);And similarly for ApplyWithSessionAndDiscriminator:
- byte[] _data = new byte[1200];
+ if (system_discriminator == null || system_discriminator.Length != 8)
+ throw new ArgumentException("system_discriminator must be 8 bytes", nameof(system_discriminator));
+ int size = IX_APPLY_WITH_SESSION_AND_DISCRIMINATOR.Length + 4 + system_discriminator.Length + 4 + args.Length;
+ byte[] _data = new byte[size];📝 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.
| public static Solana.Unity.Rpc.Models.TransactionInstruction ApplyWithDiscriminator(ApplyAccounts accounts, byte[] system_discriminator, byte[] args, PublicKey programId = null) | |
| { | |
| programId ??= new(ID); | |
| List<Solana.Unity.Rpc.Models.AccountMeta> keys = new() | |
| { | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.BoltSystem, false), | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.Authority, true), | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.InstructionSysvarAccount, false), | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.World, false) | |
| }; | |
| byte[] _data = new byte[1200]; | |
| int offset = 0; | |
| _data.WriteSpan(IX_APPLY_WITH_DISCRIMINATOR, offset); | |
| offset += 8; | |
| _data.WriteS32(system_discriminator.Length, offset); | |
| offset += 4; | |
| _data.WriteSpan(system_discriminator, offset); | |
| offset += system_discriminator.Length; | |
| _data.WriteS32(args.Length, offset); | |
| offset += 4; | |
| _data.WriteSpan(args, offset); | |
| offset += args.Length; | |
| byte[] resultData = new byte[offset]; | |
| Array.Copy(_data, resultData, offset); | |
| return new Solana.Unity.Rpc.Models.TransactionInstruction{Keys = keys, ProgramId = programId.KeyBytes, Data = resultData}; | |
| } | |
| public static Solana.Unity.Rpc.Models.TransactionInstruction ApplyWithSessionAndDiscriminator(ApplyWithSessionAccounts accounts, byte[] system_discriminator, byte[] args, PublicKey programId = null) | |
| { | |
| programId ??= new(ID); | |
| List<Solana.Unity.Rpc.Models.AccountMeta> keys = new() | |
| { | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.BoltSystem, false), | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.Authority, true), | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.InstructionSysvarAccount, false), | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.World, false), | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.SessionToken, false) | |
| }; | |
| byte[] _data = new byte[1200]; | |
| int offset = 0; | |
| _data.WriteSpan(IX_APPLY_WITH_SESSION_AND_DISCRIMINATOR, offset); | |
| offset += 8; | |
| _data.WriteS32(system_discriminator.Length, offset); | |
| offset += 4; | |
| _data.WriteSpan(system_discriminator, offset); | |
| offset += system_discriminator.Length; | |
| _data.WriteS32(args.Length, offset); | |
| offset += 4; | |
| _data.WriteSpan(args, offset); | |
| offset += args.Length; | |
| byte[] resultData = new byte[offset]; | |
| Array.Copy(_data, resultData, offset); | |
| return new Solana.Unity.Rpc.Models.TransactionInstruction{Keys = keys, ProgramId = programId.KeyBytes, Data = resultData}; | |
| } | |
| public static Solana.Unity.Rpc.Models.TransactionInstruction ApplyWithDiscriminator(ApplyAccounts accounts, byte[] system_discriminator, byte[] args, PublicKey programId = null) | |
| { | |
| programId ??= new(ID); | |
| List<Solana.Unity.Rpc.Models.AccountMeta> keys = new() | |
| { | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.BoltSystem, false), | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.Authority, true), | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.InstructionSysvarAccount, false), | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.World, false) | |
| }; | |
| if (system_discriminator == null || system_discriminator.Length != 8) | |
| throw new ArgumentException("system_discriminator must be 8 bytes", nameof(system_discriminator)); | |
| int size = IX_APPLY_WITH_DISCRIMINATOR.Length + 4 + system_discriminator.Length + 4 + args.Length; | |
| byte[] _data = new byte[size]; | |
| int offset = 0; | |
| _data.WriteSpan(IX_APPLY_WITH_DISCRIMINATOR, offset); | |
| offset += 8; | |
| _data.WriteS32(system_discriminator.Length, offset); | |
| offset += 4; | |
| _data.WriteSpan(system_discriminator, offset); | |
| offset += system_discriminator.Length; | |
| _data.WriteS32(args.Length, offset); | |
| offset += 4; | |
| _data.WriteSpan(args, offset); | |
| offset += args.Length; | |
| byte[] resultData = new byte[offset]; | |
| Array.Copy(_data, resultData, offset); | |
| return new Solana.Unity.Rpc.Models.TransactionInstruction{Keys = keys, ProgramId = programId.KeyBytes, Data = resultData}; | |
| } | |
| public static Solana.Unity.Rpc.Models.TransactionInstruction ApplyWithSessionAndDiscriminator(ApplyWithSessionAccounts accounts, byte[] system_discriminator, byte[] args, PublicKey programId = null) | |
| { | |
| programId ??= new(ID); | |
| List<Solana.Unity.Rpc.Models.AccountMeta> keys = new() | |
| { | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.BoltSystem, false), | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.Authority, true), | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.InstructionSysvarAccount, false), | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.World, false), | |
| Solana.Unity.Rpc.Models.AccountMeta.ReadOnly(accounts.SessionToken, false) | |
| }; | |
| byte[] _data = new byte[1200]; | |
| int offset = 0; | |
| _data.WriteSpan(IX_APPLY_WITH_SESSION_AND_DISCRIMINATOR, offset); | |
| offset += 8; | |
| _data.WriteS32(system_discriminator.Length, offset); | |
| offset += 4; | |
| _data.WriteSpan(system_discriminator, offset); | |
| offset += system_discriminator.Length; | |
| _data.WriteS32(args.Length, offset); | |
| offset += 4; | |
| _data.WriteSpan(args, offset); | |
| offset += args.Length; | |
| byte[] resultData = new byte[offset]; | |
| Array.Copy(_data, resultData, offset); | |
| return new Solana.Unity.Rpc.Models.TransactionInstruction{Keys = keys, ProgramId = programId.KeyBytes, Data = resultData}; | |
| } |
🤖 Prompt for AI Agents
In clients/csharp/Solana.Unity.Bolt/WorldProgram/World.cs around lines 254 to
307, the two methods allocate a fixed 1200-byte buffer and do not validate the
system_discriminator length; change both ApplyWithDiscriminator and
ApplyWithSessionAndDiscriminator to first validate that
system_discriminator.Length == 8 (throw ArgumentException/ArgumentOutOfRange if
not), compute the exact required buffer size (8 + 4 +
system_discriminator.Length + 4 + args.Length plus any initial discriminator
bytes) or use a resizable construct (MemoryStream/List<byte>), allocate only the
necessary buffer, then write into it and copy the exact-length result; apply the
same validation and sizing logic to both methods and also guard against
negative/too-large args.Length where appropriate.
| generate_instructions(&mut program, &type_.ident, None); | ||
| if attributes.delegate { | ||
| inject_delegate_items(&mut program, vec![(type_.ident.clone(), "".to_string())]); | ||
| } | ||
| generate_update(&mut program); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Propagate component_id into codegen and delegate injection.
Passing None/"" drops the component name used for PDA seeds and discriminators in bundles. This can mis-route or collide.
Apply this diff:
- generate_instructions(&mut program, &type_.ident, None);
+ let component_name =
+ if attributes.component_id.is_empty() { None } else { Some(&attributes.component_id) };
+ generate_instructions(&mut program, &type_.ident, component_name);
@@
- inject_delegate_items(&mut program, vec![(type_.ident.clone(), "".to_string())]);
+ inject_delegate_items(&mut program, vec![(type_.ident.clone(), attributes.component_id.clone())]);📝 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.
| generate_instructions(&mut program, &type_.ident, None); | |
| if attributes.delegate { | |
| inject_delegate_items(&mut program, vec![(type_.ident.clone(), "".to_string())]); | |
| } | |
| generate_update(&mut program); | |
| let component_name = | |
| if attributes.component_id.is_empty() { None } else { Some(&attributes.component_id) }; | |
| generate_instructions(&mut program, &type_.ident, component_name); | |
| if attributes.delegate { | |
| inject_delegate_items(&mut program, vec![(type_.ident.clone(), attributes.component_id.clone())]); | |
| } | |
| generate_update(&mut program); |
🤖 Prompt for AI Agents
In crates/bolt-lang/attribute/src/component/mod.rs around lines 19–23, the code
currently calls generate_instructions(..., None) and inject_delegate_items(...,
"".to_string()), which drops the component identifier used for PDA
seeds/discriminators; instead pass the actual component_id through both calls.
Replace the None with Some(component_id.clone() or reference) when calling
generate_instructions, and supply the real component_id string (e.g.,
component_id.to_string() or component_id.clone()) in the tuple passed to
inject_delegate_items so the generated code and delegate injection use the
correct component identifier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (6)
crates/bolt-lang/attribute/extra-accounts/src/lib.rs (1)
89-91: Fully qualify Result and ErrorCode to avoid scope collisions.The unqualified
ResultandErrorCodetypes can collide with caller scopes during macro expansion.Apply this diff:
- fn #field_name(&self) -> Result<&'c AccountInfo<'info>> { - self.remaining_accounts.get(<#type_path as bolt_lang::NumberOfComponents>::NUMBER_OF_COMPONENTS + #index).ok_or_else(|| ErrorCode::ConstraintAccountIsNone.into()) + fn #field_name(&self) -> anchor_lang::Result<&'c AccountInfo<'info>> { + self.remaining_accounts + .get(<#type_path as bolt_lang::NumberOfComponents>::NUMBER_OF_COMPONENTS + #index) + .ok_or_else(|| anchor_lang::error::ErrorCode::ConstraintAccountIsNone.into()) }crates/bolt-lang/attribute/src/system/mod.rs (5)
79-82: Improve error handling in panic path.The panic itself calls
unwrap()oncontext_struct_namewhich may beNone, potentially causing a panic-within-panic that obscures the actual error.Apply this diff:
} else { panic!( "Could not find the component bundle: {} in the module", - extractor.context_struct_name.unwrap() + extractor.context_struct_name.unwrap_or_else(|| "unknown".to_string()) ); }
92-96: Improve error message in panic path.Using
unwrap_or_default()produces an empty string whencontext_struct_nameisNone, resulting in an unhelpful error message.Apply this diff:
if extractor.field_count.is_none() { panic!( "Could not find the component bundle: {} in the module", - extractor.context_struct_name.unwrap_or_default() + extractor.context_struct_name.as_deref().unwrap_or("unknown") ); }
107-184: Consider extracting inline visitor structs.The
SystemRenameandExecRenamevisitor structs defined inline increase cognitive complexity and hinder testability. Extracting them as module-level private structs would improve code organization.
343-343: Handle parse errors gracefully.Direct
unwrap()onparse2()could panic if token generation fails, though this is unlikely given the quote! macro produces valid syntax.Apply this diff:
- content.push(syn::parse2(initialize_extra_accounts).unwrap()); + content.push(syn::parse2(initialize_extra_accounts).expect("Failed to parse init_extra_accounts function"));
379-381: Directunwrap()could panic if path has no segments.Though
Type::Pathin syn typically has at least one segment, usingunwrap()here is risky and could panic in edge cases.Apply this diff:
Type::Path(mut tp) => { - let seg = tp.path.segments.last_mut().unwrap(); + let seg = tp.path.segments.last_mut().expect("Type path must have at least one segment"); match &mut seg.arguments {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
crates/bolt-lang/attribute/extra-accounts/Cargo.toml(1 hunks)crates/bolt-lang/attribute/extra-accounts/src/lib.rs(4 hunks)crates/bolt-lang/attribute/src/system/mod.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
crates/bolt-lang/attribute/src/system/mod.rs (3)
crates/bolt-lang/attribute/system-input/src/lib.rs (2)
syn(43-43)system_input(20-133)crates/bolt-lang/attribute/src/bundle/mod.rs (1)
process(10-80)crates/bolt-lang/src/lib.rs (1)
parse_args(50-55)
crates/bolt-lang/attribute/extra-accounts/src/lib.rs (1)
crates/bolt-lang/attribute/system-input/src/lib.rs (1)
syn(43-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: setup
- GitHub Check: setup
🔇 Additional comments (14)
crates/bolt-lang/attribute/extra-accounts/Cargo.toml (1)
18-18: heck dependency is correctly used and properly integrated.The dependency is imported and used in
crates/bolt-lang/attribute/extra-accounts/src/lib.rsfor case conversion (line 32:extra_accounts_struct_name.to_string().to_pascal_case()). The version is properly defined in the workspaceCargo.tomlat0.5.0, with no conflicts or redundancies across attribute crates. The addition aligns with the PR's identifier transformation goals.crates/bolt-lang/attribute/extra-accounts/src/lib.rs (3)
1-1: LGTM! Import additions support the new functionality.The
ToPascalCaseandSpanimports are correctly used for trait name generation and identifier construction.Also applies to: 4-4
77-82: LGTM! Dynamic trait name generation enables type-specific extensions.The generated trait name based on the struct name allows multiple extra_accounts types to coexist without conflicts.
95-99: LGTM! Trait implementation correctly uses generated names and type path.The implementation properly matches the trait declaration with the generated name and concrete type path, enabling type-specific context extensions.
crates/bolt-lang/attribute/src/system/mod.rs (10)
1-19: LGTM: Clean structure and imports.The imports and state structs are well-organized for the AST transformation pipeline.
21-35: LGTM: Wrapper generation is correct.The bolt_execute wrapper properly bridges VariadicBoltComponents to system-specific Components and delegates to the renamed execute function.
43-77: LGTM: Standalone system processing logic is sound.The function correctly extracts bundle information, injects necessary boilerplate (use super::*, VariadicBoltComponents), generates the wrapper, and applies transformations.
86-91: LGTM: Bundle transformation orchestration is well-structured.The function correctly handles the bundle context by removing system attributes, extracting bundle info, applying transformations, and generating appropriately-named wrappers to avoid collisions.
Also applies to: 98-106, 184-195
197-263: LGTM: Visitor implementation correctly transforms system modules.The visitor properly handles expression traversal, return type conversion, and attribute enrichment for system_input and extra_accounts.
Also applies to: 265-342, 344-346
349-378: LGTM: Lifetime injection logic is sophisticated and correct.The function properly injects Anchor-required lifetimes and extends the Context type to carry component information, handling both empty and existing generic arguments.
Also applies to: 382-420
422-457: LGTM: Type checking utilities are comprehensive.The functions correctly identify
Result<Vec<Vec<u8>>>patterns and validate inner Vec types.
459-486: LGTM: Expression extraction correctly identifies Ok patterns.The function properly handles both direct
Ok(...)calls andreturn Ok(...)expressions to extract inner values for transformation.
488-524: LGTM: Argument transformation correctly integrates deserialization.The function safely checks argument count, renames parameters, changes types to
Vec<u8>, and inserts theparse_argscall to deserialize at runtime, aligning with the Bolt runtime's serialization approach.
526-561: LGTM: Extractor correctly identifies component bundle metadata.The visitor properly extracts the context struct name from the execute function's Context parameter and counts its fields, providing the information needed for subsequent transformations. The use of
expect()at line 535 provides a clear error message.
| pub fn extra_accounts(attr: TokenStream, item: TokenStream) -> TokenStream { | ||
| let attr = parse_macro_input!(attr as syn::MetaNameValue); | ||
| let literal = if let syn::Lit::Str(lit_str) = attr.lit { | ||
| lit_str.value() | ||
| } else { | ||
| panic!("Invalid literal"); | ||
| }; | ||
| let type_path = syn::parse_str::<syn::TypePath>(&literal).expect("Invalid type path"); | ||
| let input = parse_macro_input!(item as ItemStruct); | ||
| let extra_accounts_struct_name = &input.ident; | ||
| let context_extension_name = syn::Ident::new( | ||
| &format!( | ||
| "ContextExtensions{}", | ||
| extra_accounts_struct_name.to_string().to_pascal_case() | ||
| ), | ||
| Span::call_site(), | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace panics with proper compile errors for better diagnostics.
Proc macros should emit compile_error! tokens instead of panicking to provide clearer error messages at the call site.
Apply this diff to improve error handling:
pub fn extra_accounts(attr: TokenStream, item: TokenStream) -> TokenStream {
- let attr = parse_macro_input!(attr as syn::MetaNameValue);
- let literal = if let syn::Lit::Str(lit_str) = attr.lit {
- lit_str.value()
- } else {
- panic!("Invalid literal");
+ let attr = match syn::parse::<syn::MetaNameValue>(attr.clone()) {
+ Ok(attr) => attr,
+ Err(e) => return TokenStream::from(quote! { compile_error!(#e.to_string()); }),
+ };
+ let literal = if let syn::Lit::Str(lit_str) = &attr.lit {
+ lit_str.value()
+ } else {
+ return TokenStream::from(quote! {
+ compile_error!("extra_accounts attribute requires a string literal type path");
+ });
};
- let type_path = syn::parse_str::<syn::TypePath>(&literal).expect("Invalid type path");
+ let type_path = match syn::parse_str::<syn::TypePath>(&literal) {
+ Ok(tp) => tp,
+ Err(e) => return TokenStream::from(quote! {
+ compile_error!(concat!("Invalid type path: ", #literal));
+ }),
+ };📝 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 fn extra_accounts(attr: TokenStream, item: TokenStream) -> TokenStream { | |
| let attr = parse_macro_input!(attr as syn::MetaNameValue); | |
| let literal = if let syn::Lit::Str(lit_str) = attr.lit { | |
| lit_str.value() | |
| } else { | |
| panic!("Invalid literal"); | |
| }; | |
| let type_path = syn::parse_str::<syn::TypePath>(&literal).expect("Invalid type path"); | |
| let input = parse_macro_input!(item as ItemStruct); | |
| let extra_accounts_struct_name = &input.ident; | |
| let context_extension_name = syn::Ident::new( | |
| &format!( | |
| "ContextExtensions{}", | |
| extra_accounts_struct_name.to_string().to_pascal_case() | |
| ), | |
| Span::call_site(), | |
| ); | |
| pub fn extra_accounts(attr: TokenStream, item: TokenStream) -> TokenStream { | |
| let attr = match syn::parse::<syn::MetaNameValue>(attr.clone()) { | |
| Ok(attr) => attr, | |
| Err(e) => return TokenStream::from(quote! { compile_error!(#e.to_string()); }), | |
| }; | |
| let literal = if let syn::Lit::Str(lit_str) = &attr.lit { | |
| lit_str.value() | |
| } else { | |
| return TokenStream::from(quote! { | |
| compile_error!("extra_accounts attribute requires a string literal type path"); | |
| }); | |
| }; | |
| let type_path = match syn::parse_str::<syn::TypePath>(&literal) { | |
| Ok(tp) => tp, | |
| Err(e) => return TokenStream::from(quote! { | |
| compile_error!(concat!("Invalid type path: ", #literal)); | |
| }), | |
| }; | |
| let input = parse_macro_input!(item as ItemStruct); | |
| let extra_accounts_struct_name = &input.ident; | |
| let context_extension_name = syn::Ident::new( | |
| &format!( | |
| "ContextExtensions{}", | |
| extra_accounts_struct_name.to_string().to_pascal_case() | |
| ), | |
| Span::call_site(), | |
| ); |
🤖 Prompt for AI Agents
In crates/bolt-lang/attribute/extra-accounts/src/lib.rs around lines 27 to 43,
replace direct panics and expect calls with proper proc-macro compile errors:
when attr.lit is not a string or when parse_str::<TypePath> fails, construct a
syn::Error (or create a TokenStream containing compile_error! with a helpful
message) using the relevant span (e.g., attr.span() or
lit.span()/input.ident.span()) and return its to_compile_error().into() so the
compiler shows a diagnostic at the call site instead of unwinding; ensure every
panic/expect is converted to an early return of a TokenStream error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/publish-bolt-crates.yml(2 hunks).github/workflows/publish-bolt-sdk.yml(1 hunks).github/workflows/run-tests.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: notdanilo
Repo: magicblock-labs/bolt PR: 207
File: crates/bolt-lang/attribute/src/component/attributes.rs:35-56
Timestamp: 2025-10-15T16:28:50.695Z
Learning: In the Bolt Lang attribute system (crates/bolt-lang/attribute), component_id in the #[component] attribute is optional and can be empty by design. When not provided, it defaults to an empty string in the Attributes struct, with the code generator providing fallback logic (e.g., using the type name) at a later stage rather than during attribute parsing.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint (yarn)
- GitHub Check: lint (clippy)
- GitHub Check: test
🔇 Additional comments (3)
.github/workflows/run-tests.yml (1)
92-92: VerifyEXAMPLE_BUNDLEsecret is configured in GitHub repository settings.The code change correctly follows the established keypair setup pattern. However, automated secret verification failed due to permission restrictions. Manually verify that
EXAMPLE_BUNDLEis configured by visiting your repository's Settings > Secrets and variables > Actions, or confirm the deployment succeeds during the test workflow run..github/workflows/publish-bolt-sdk.yml (1)
147-147: Verify thatEXAMPLE_BUNDLEsecret is configured in repository settings.This line adds the example bundle deployment keypair without validating that the
EXAMPLE_BUNDLEsecret is set. If the secret is missing or empty, the workflow will write an invalid keypair file, potentially causing silent failures during subsequent build/test steps.Confirm that:
- The
EXAMPLE_BUNDLEsecret is set in the repository's GitHub settings.- The secret contains a valid Solana keypair JSON.
- Users/maintainers are aware of this new secret requirement (documentation update may be needed).
.github/workflows/publish-bolt-crates.yml (1)
142-142: Verify thatEXAMPLE_BUNDLEsecret is configured in repository settings.Same concern as in publish-bolt-sdk.yml: this line adds the example bundle deployment keypair without validating that the
EXAMPLE_BUNDLEsecret is set. If missing or empty, downstream build/test steps may fail silently.Confirm that the
EXAMPLE_BUNDLEsecret is set and contains a valid Solana keypair JSON.
| -p bolt-system \ | ||
| -p bolt-component \ | ||
| -p bolt-attribute-bolt-arguments \ | ||
| -p bolt-attribute-bolt-bundle \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify that bolt-attribute-bolt-bundle package is properly configured in the workspace.
The new package is added to the crates.io publish list. Confirm that:
bolt-attribute-bolt-bundleexists in the Cargo workspace and is included in workspace members.- The package has proper
Cargo.tomlconfiguration (name, version, publish settings, etc.). - The package is ready for public release on crates.io.
🏁 Script executed:
#!/bin/bash
# Description: Verify bolt-attribute-bolt-bundle package exists and is configured
# Expected: Package directory exists and Cargo.toml is properly configured
# Find the package
fd -t d 'bolt-attribute-bolt-bundle' --max-depth 2
# Check workspace configuration
rg -A 20 '^\[workspace\]' Cargo.toml | head -30
# Check if the package is listed in members
rg '"bolt-attribute-bolt-bundle"' Cargo.tomlLength of output: 518
bolt-attribute-bolt-bundle is not configured in the workspace and must be added before publishing.
The package does not exist in the workspace members list. Verification shows:
- The package directory does not exist
- The package is not listed in
Cargo.tomlworkspace members (only "crates/bolt-cli", "crates/bolt-lang", "crates/programs/world", "crates/types", and "examples/*" are present) - The package is not referenced in workspace dependencies
Before adding this package to the publish workflow, you must:
- Create the package directory structure
- Add the package to the
membersarray in the rootCargo.toml - Ensure its
Cargo.tomlis properly configured with name, version, and publish settings
🤖 Prompt for AI Agents
.github/workflows/publish-bolt-crates.yml around line 184: the workflow
references a package name "bolt-attribute-bolt-bundle" that does not exist in
the workspace; create the missing crate and register it before including it in
the publish step — create the package directory (e.g.,
crates/bolt-attribute-bolt-bundle) with a valid Cargo.toml, add the new package
name to the root Cargo.toml workspace.members array, ensure the crate's
Cargo.toml contains the correct package name, version, edition and publish
settings (and any workspace dependency entries if required), then re-run or
update the workflow entry to reference the newly added workspace member.
Problem
Deployment cost of projects with several components and systems programs might get too high. We want to allow developers to bundle systems and components in a single program to reduce deployment costs.
Solution
Introduce the
#[bundle]to allow to ship several#[component]s and#[system]s in a single program.To implement this, we need to add discriminators for components and systems in the program.
The biggest API change is in the delegation instruction, because now we need to send the seeds as a parameter.
Greptile Overview
Updated On: 2025-10-06 07:08:22 UTC
Summary
This PR introduces a major architectural feature called "bundling" that allows developers to combine multiple `#[component]`s and `#[system]`s into a single Solana program, addressing the high deployment costs of approximately 1.7 SOL per component/system. The solution adds a new `#[bundle]` procedural macro that generates unified programs with discriminator-based routing to distinguish between different components and systems within the same program.The implementation involves significant changes across the entire codebase:
Core Architecture Changes:
bolt-componentandbolt-systemprograms, replacing them with bundle functionalitybolt-attributecrate to enable code reuse between standalone and bundled deploymentsBundle Implementation:
#[bundle]macro processes modules containing mixed component structs and system modulesClient Integration:
ComponentandSystemclasses provide abstractions for working with both standalone and bundled entitiesGetDiscriminatorfunctions implement Anchor's standard SHA-256-based discriminator generationDevelopment Workflow:
bolt bundle <name>creates bundle templates inprograms-ecs/bundles/The bundling feature maintains backward compatibility with existing standalone components and systems while providing a cost-effective deployment option for projects with multiple ECS entities. The architecture enables the same component and system logic to work in both contexts through shared instruction modules and unified API interfaces.
Sequence Diagram
sequenceDiagram participant User participant BoltCLI participant BundleTemplate participant BundleAttribute participant ComponentGen participant SystemGen participant AnchorProgram User->>BoltCLI: "bolt bundle example-bundle" BoltCLI->>BundleTemplate: "create_bundle_template()" BundleTemplate->>BundleTemplate: "generate bundle lib.rs with #[bundle]" BundleTemplate-->>BoltCLI: "bundle files created" User->>AnchorProgram: "anchor build" AnchorProgram->>BundleAttribute: "process(#[bundle] mod)" BundleAttribute->>BundleAttribute: "parse bundle module content" loop "for each item in bundle" alt "item is #[component] struct" BundleAttribute->>ComponentGen: "generate_implementation()" ComponentGen->>ComponentGen: "generate initialize/update/destroy functions" ComponentGen->>ComponentGen: "add component name as PDA seed suffix" ComponentGen-->>BundleAttribute: "component instructions generated" else "item is #[system] mod" BundleAttribute->>SystemGen: "transform_module_for_bundle()" SystemGen->>SystemGen: "rename Components to {name}Components" SystemGen->>SystemGen: "generate {name}_bolt_execute wrapper" SystemGen-->>BundleAttribute: "system instructions generated" end end BundleAttribute->>BundleAttribute: "wrap everything in #[program] module" BundleAttribute-->>AnchorProgram: "unified program module" User->>User: "deploy bundle as single program" Note over User,AnchorProgram: Bundle allows multiple components/systems<br/>in one program to reduce deployment costsSummary by CodeRabbit
New Features
Improvements