Repository navigation
feat: remove noop dependency - #721
Conversation
📝 WalkthroughWalkthroughAdds a new public enum variant Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-12-03T09:33:48.707ZApplied to files:
📚 Learning: 2025-11-12T09:46:27.553ZApplied to files:
📚 Learning: 2025-11-07T13:09:52.253ZApplied to files:
📚 Learning: 2025-11-07T14:20:31.457ZApplied to files:
📚 Learning: 2025-12-03T09:36:01.527ZApplied to files:
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 |
Manual Deploy AvailableYou can trigger a manual deploy of this PR branch to testnet: Alternative: Comment
Comment updated automatically when the PR is synchronized. |
|
@lucacillario this removes the need to upload the noop program when deploying |
bmuddha
left a comment
bmuddha
left a comment
There was a problem hiding this comment.
LGTM, please also remove noop program from test-kit
| // ----------------- | ||
| // Noop | ||
| // ----------------- | ||
| pub fn noop( |
There was a problem hiding this comment.
nit: Not sure why this is even needed? It's not used anywhere
There was a problem hiding this comment.
Repeated the style of util functions having ix and tx but agreed, addressed in 78f489d
|
The noop program is already removed from test-kit |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
programs/magicblock/src/utils/instruction_utils.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 650
File: magicblock-chainlink/src/submux/subscription_task.rs:13-99
Timestamp: 2025-11-20T08:57:07.217Z
Learning: In the magicblock-validator repository, avoid posting review comments that merely confirm code is correct or matches intended behavior without providing actionable feedback, suggestions for improvement, or identifying potential issues. Such confirmatory comments are considered unhelpful noise by the maintainers.
🧬 Code graph analysis (1)
programs/magicblock/src/utils/instruction_utils.rs (1)
magicblock-task-scheduler/src/service.rs (1)
noop_instruction(315-318)
| // ----------------- | ||
| // Noop | ||
| // ----------------- | ||
| pub fn noop_instruction(data: u64) -> Instruction { | ||
| Instruction::new_with_bincode( | ||
| crate::id(), | ||
| &MagicBlockInstruction::Noop(data), | ||
| vec![], | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify noop_instruction semantics with a short doc comment
The implementation looks fine, but the intent of data and the empty account metas isn’t obvious to a reader unfamiliar with the Noop variant. A brief doc comment would make it clear that the payload is just a nonce/uniqueness seed and that the processor ignores it and expects no accounts.
You could do something like:
- // -----------------
- // Noop
- // -----------------
- pub fn noop_instruction(data: u64) -> Instruction {
+ // -----------------
+ // Noop
+ // -----------------
+ /// Creates a MagicBlock `Noop` instruction.
+ ///
+ /// `data` is an arbitrary nonce used only to make the instruction
+ /// unique; it is ignored by the on-chain processor and no accounts
+ /// are required for this instruction.
+ pub fn noop_instruction(data: u64) -> Instruction {
Instruction::new_with_bincode(
crate::id(),
&MagicBlockInstruction::Noop(data),
vec![],
)
}This should help future callers understand how to use this helper correctly without having to inspect the processor logic. Based on learnings, avoiding pure LGTM comments.
📝 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.
| // ----------------- | |
| // Noop | |
| // ----------------- | |
| pub fn noop_instruction(data: u64) -> Instruction { | |
| Instruction::new_with_bincode( | |
| crate::id(), | |
| &MagicBlockInstruction::Noop(data), | |
| vec![], | |
| ) | |
| } | |
| // ----------------- | |
| // Noop | |
| // ----------------- | |
| /// Creates a MagicBlock `Noop` instruction. | |
| /// | |
| /// `data` is an arbitrary nonce used only to make the instruction | |
| /// unique; it is ignored by the on-chain processor and no accounts | |
| /// are required for this instruction. | |
| pub fn noop_instruction(data: u64) -> Instruction { | |
| Instruction::new_with_bincode( | |
| crate::id(), | |
| &MagicBlockInstruction::Noop(data), | |
| vec![], | |
| ) | |
| } |
🤖 Prompt for AI Agents
In programs/magicblock/src/utils/instruction_utils.rs around lines 281 to 290,
the noop_instruction helper lacks documentation explaining the semantics of its
`data` parameter and the empty account vec; add a short doc comment above the
function that states: the `data` argument is a payload used only as a
nonce/uniqueness seed (ignored by the processor), the Noop variant does not
require or use any account metas so the returned Instruction intentionally has
an empty account list, and callers should not expect any on-chain effects —
update the comment to be concise and place it directly above the pub fn
noop_instruction signature.
3 participants
Fixes #563
Removes the dependency on an external noop program in favor of a noop magic program instruction.
Summary by CodeRabbit
New Features
Tests
Chores
Bug Fixes / Stability
✏️ Tip: You can customize this high-level summary in your review settings.