Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions magicblock-magic-program-api/src/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ pub enum MagicBlockInstruction {
/// # Account references
/// - **0.** `[SIGNER]` Validator authority
EnableExecutableCheck,

/// Noop instruction
Noop(u64),
}

impl MagicBlockInstruction {
Expand Down
17 changes: 5 additions & 12 deletions magicblock-task-scheduler/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ use magicblock_core::link::transactions::{
use magicblock_ledger::LatestBlock;
use magicblock_program::{
args::{CancelTaskRequest, TaskRequest},
instruction_utils::InstructionUtils,
validator::{validator_authority, validator_authority_id},
};
use solana_sdk::{
instruction::Instruction, message::Message, pubkey::Pubkey,
signature::Signature, transaction::Transaction,
instruction::Instruction, message::Message, signature::Signature,
transaction::Transaction,
};
use tokio::{select, task::JoinHandle, time::Duration};
use tokio_util::{
Expand All @@ -30,9 +31,6 @@ use crate::{
errors::{TaskSchedulerError, TaskSchedulerResult},
};

const NOOP_PROGRAM_ID: Pubkey =
Pubkey::from_str_const("noopb9bkMVfRPU8AsbpTUg8AQkHtKwMYZiFUjNRtMmV");

pub struct TaskSchedulerService {
/// Database for persisting tasks
db: SchedulerDatabase,
Expand Down Expand Up @@ -308,13 +306,8 @@ impl TaskSchedulerService {
let blockhash = self.block.load().blockhash;
// Execute unsigned transactions
// We prepend a noop instruction to make each transaction unique.
let noop_instruction = Instruction::new_with_bytes(
NOOP_PROGRAM_ID,
&self
.tx_counter
.fetch_add(1, Ordering::Relaxed)
.to_le_bytes(),
vec![],
let noop_instruction = InstructionUtils::noop_instruction(
self.tx_counter.fetch_add(1, Ordering::Relaxed),
);
let tx = Transaction::new(
&[validator_authority()],
Expand Down
3 changes: 3 additions & 0 deletions magicblock-task-scheduler/tests/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,9 @@ pub async fn test_cancel_task() -> TaskSchedulerResult<()> {
result
);

// Wait for the cancel to be processed
tokio::time::sleep(Duration::from_millis(interval as u64)).await;

let value_at_cancel = env
.get_account(account.pubkey())
.data()
Expand Down
1 change: 1 addition & 0 deletions programs/magicblock/src/magicblock_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ declare_process_instruction!(
EnableExecutableCheck => {
process_toggle_executable_check(signers, invoke_context, true)
}
Noop(_) => Ok(()),
}
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -154,15 +154,13 @@ mod test {

use super::*;
use crate::{
test_utils::{
process_instruction, COUNTER_PROGRAM_ID, NOOP_PROGRAM_ID,
},
test_utils::{process_instruction, COUNTER_PROGRAM_ID},
utils::instruction_utils::InstructionUtils,
validator::generate_validator_authority_if_needed,
};

fn create_simple_ix() -> Instruction {
Instruction::new_with_borsh(NOOP_PROGRAM_ID, b"test noop", vec![])
InstructionUtils::noop_instruction(0)
}

fn create_complex_ix(
Expand Down
2 changes: 0 additions & 2 deletions programs/magicblock/src/test_utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ use super::*;
use crate::validator;

pub const AUTHORITY_BALANCE: u64 = u64::MAX / 2;
pub const NOOP_PROGRAM_ID: Pubkey =
Pubkey::from_str_const("noopb9bkMVfRPU8AsbpTUg8AQkHtKwMYZiFUjNRtMmV");
pub const COUNTER_PROGRAM_ID: Pubkey =
Pubkey::from_str_const("2jQZbSfAfqT5nZHGrLpDG2vXuEGtTgZYnNy7AZEjMCYz");

Expand Down
11 changes: 11 additions & 0 deletions programs/magicblock/src/utils/instruction_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,17 @@ impl InstructionUtils {
)
}

// -----------------
// Noop
// -----------------
pub fn noop_instruction(data: u64) -> Instruction {
Instruction::new_with_bincode(
crate::id(),
&MagicBlockInstruction::Noop(data),
vec![],
)
}
Comment on lines +281 to +290

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 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.

Suggested change
// -----------------
// 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.


// -----------------
// Utils
// -----------------
Expand Down
4 changes: 0 additions & 4 deletions test-integration/configs/schedule-task.devnet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,5 @@ path = "../schedulecommit/elfs/dlp.so"
id = "DmnRGfyyftzacFb1XadYhWF6vWqXwtQk5tbr6XgR3BA1"
path = "../schedulecommit/elfs/mdp.so"

[[programs]]
id = "noopb9bkMVfRPU8AsbpTUg8AQkHtKwMYZiFUjNRtMmV"
path = "../programs/noop/noop.so"

[metrics]
address = "0.0.0.0:9000"
Binary file removed test-integration/programs/noop/noop.so
Binary file not shown.
39 changes: 1 addition & 38 deletions test-integration/test-task-scheduler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,10 @@ use program_flexi_counter::instruction::{
create_delegate_ix_with_commit_frequency_ms, create_init_ix,
};
use solana_sdk::{
hash::Hash, instruction::Instruction, pubkey::Pubkey, signature::Keypair,
signer::Signer, transaction::Transaction,
signature::Keypair, signer::Signer, transaction::Transaction,
};
use tempfile::TempDir;

pub const NOOP_PROGRAM_ID: Pubkey =
Pubkey::from_str_const("noopb9bkMVfRPU8AsbpTUg8AQkHtKwMYZiFUjNRtMmV");

pub const TASK_SCHEDULER_TICK_MILLIS: u64 = 50;

pub fn setup_validator() -> (TempDir, Child, IntegrationTestContext) {
Expand Down Expand Up @@ -126,36 +122,3 @@ pub fn create_delegated_counter(
// Wait for account to be delegated
expect!(ctx.wait_for_delta_slot_ephem(10), validator);
}

pub fn send_noop_tx(
ctx: &IntegrationTestContext,
payer: &Keypair,
validator: &mut Child,
) -> Hash {
// Noop tx to make sure the noop program is cloned
let ephem_blockhash = expect!(
ctx.try_ephem_client().and_then(|client| client
.get_latest_blockhash()
.map_err(|e| anyhow::anyhow!(
"Failed to get latest blockhash: {}",
e
))),
validator
);
let noop_instruction =
Instruction::new_with_bytes(NOOP_PROGRAM_ID, &[0], vec![]);
expect!(
ctx.send_transaction_ephem(
&mut Transaction::new_signed_with_payer(
&[noop_instruction],
Some(&payer.pubkey()),
&[&payer],
ephem_blockhash,
),
&[payer]
),
validator
);

ephem_blockhash
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ use solana_sdk::{
native_token::LAMPORTS_PER_SOL, signature::Keypair, signer::Signer,
transaction::Transaction,
};
use test_task_scheduler::{
create_delegated_counter, send_noop_tx, setup_validator,
};
use test_task_scheduler::{create_delegated_counter, setup_validator};
use tokio::runtime::Runtime;

#[test]
Expand All @@ -29,8 +27,8 @@ fn test_cancel_ongoing_task() {

create_delegated_counter(&ctx, &payer, &mut validator, 0);

// Noop tx to make sure the noop program is cloned
let ephem_blockhash = send_noop_tx(&ctx, &payer, &mut validator);
let ephem_blockhash =
expect!(ctx.try_get_latest_blockhash_ephem(), validator);

// Schedule a task
let task_id = 3;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ use solana_sdk::{
native_token::LAMPORTS_PER_SOL, signature::Keypair, signer::Signer,
transaction::Transaction,
};
use test_task_scheduler::{
create_delegated_counter, send_noop_tx, setup_validator,
};
use test_task_scheduler::{create_delegated_counter, setup_validator};
use tokio::runtime::Runtime;

#[test]
Expand All @@ -29,8 +27,8 @@ fn test_reschedule_task() {

create_delegated_counter(&ctx, &payer, &mut validator, 0);

// Noop tx to make sure the noop program is cloned
let ephem_blockhash = send_noop_tx(&ctx, &payer, &mut validator);
let ephem_blockhash =
expect!(ctx.try_get_latest_blockhash_ephem(), validator);

// Schedule a task
let task_id = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ use solana_sdk::{
native_token::LAMPORTS_PER_SOL, signature::Keypair, signer::Signer,
transaction::Transaction,
};
use test_task_scheduler::{
create_delegated_counter, send_noop_tx, setup_validator,
};
use test_task_scheduler::{create_delegated_counter, setup_validator};
use tokio::runtime::Runtime;

// Test that a task with an error is unscheduled
Expand All @@ -30,8 +28,8 @@ fn test_schedule_error() {

create_delegated_counter(&ctx, &payer, &mut validator, 0);

// Noop tx to make sure the noop program is cloned
let ephem_blockhash = send_noop_tx(&ctx, &payer, &mut validator);
let ephem_blockhash =
expect!(ctx.try_get_latest_blockhash_ephem(), validator);

// Schedule a task
let task_id = 2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ use solana_sdk::{
native_token::LAMPORTS_PER_SOL, signature::Keypair, signer::Signer,
transaction::Transaction,
};
use test_task_scheduler::{
create_delegated_counter, send_noop_tx, setup_validator,
};
use test_task_scheduler::{create_delegated_counter, setup_validator};
use tokio::runtime::Runtime;

#[test]
Expand All @@ -29,8 +27,8 @@ fn test_schedule_task() {

create_delegated_counter(&ctx, &payer, &mut validator, 0);

// Noop tx to make sure the noop program is cloned
let ephem_blockhash = send_noop_tx(&ctx, &payer, &mut validator);
let ephem_blockhash =
expect!(ctx.try_get_latest_blockhash_ephem(), validator);

// Schedule a task
let task_id = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ use solana_sdk::{
signer::Signer,
transaction::{Transaction, TransactionError},
};
use test_task_scheduler::{
create_delegated_counter, send_noop_tx, setup_validator,
};
use test_task_scheduler::{create_delegated_counter, setup_validator};

/// Test that a task can be scheduled and executed when it has multiple signers
#[test]
Expand All @@ -24,8 +22,8 @@ fn test_schedule_task_signed() {

create_delegated_counter(&ctx, &payer, &mut validator, 0);

// Noop tx to make sure the noop program is cloned
let ephem_blockhash = send_noop_tx(&ctx, &payer, &mut validator);
let ephem_blockhash =
expect!(ctx.try_get_latest_blockhash_ephem(), validator);

// Schedule a task
let task_id = 4;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ use solana_sdk::{
native_token::LAMPORTS_PER_SOL, signature::Keypair, signer::Signer,
transaction::Transaction,
};
use test_task_scheduler::{
create_delegated_counter, send_noop_tx, setup_validator,
};
use test_task_scheduler::{create_delegated_counter, setup_validator};

#[test]
fn test_scheduled_commits() {
Expand All @@ -23,8 +21,8 @@ fn test_scheduled_commits() {
validator
);

// Noop tx to make sure the noop program is cloned
let ephem_blockhash = send_noop_tx(&ctx, &payer, &mut validator);
let ephem_blockhash =
expect!(ctx.try_get_latest_blockhash_ephem(), validator);

let commit_frequency_ms = 400;
create_delegated_counter(&ctx, &payer, &mut validator, commit_frequency_ms);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ use solana_sdk::{
native_token::LAMPORTS_PER_SOL, signature::Keypair, signer::Signer,
transaction::Transaction,
};
use test_task_scheduler::{
create_delegated_counter, send_noop_tx, setup_validator,
};
use test_task_scheduler::{create_delegated_counter, setup_validator};
use tokio::runtime::Runtime;

#[test]
Expand All @@ -34,8 +32,8 @@ fn test_unauthorized_reschedule() {
create_delegated_counter(&ctx, &payer, &mut validator, 0);
create_delegated_counter(&ctx, &different_payer, &mut validator, 0);

// Noop tx to make sure the noop program is cloned
let ephem_blockhash = send_noop_tx(&ctx, &payer, &mut validator);
let ephem_blockhash =
expect!(ctx.try_get_latest_blockhash_ephem(), validator);

// Schedule a task
let task_id = 1;
Expand Down
9 changes: 0 additions & 9 deletions test-kit/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ use solana_transaction::Transaction;
use solana_transaction_status_client_types::TransactionStatusMeta;
use tempfile::TempDir;

const NOOP_PROGRAM_ID: Pubkey =
Pubkey::from_str_const("noopb9bkMVfRPU8AsbpTUg8AQkHtKwMYZiFUjNRtMmV");

/// A simulated validator backend for integration tests.
///
/// This struct encapsulates all the core components of a validator, including
Expand Down Expand Up @@ -136,12 +133,6 @@ impl ExecutionTestEnv {
"../programs/elfs/guinea.so".into(),
)])
.expect("failed to load test programs into test env");
scheduler_state
.load_upgradeable_programs(&[(
NOOP_PROGRAM_ID,
"../test-integration/programs/noop/noop.so".into(),
)])
.expect("failed to load test programs into test env");

// Start the transaction processing backend.
TransactionScheduler::new(1, scheduler_state).spawn();
Expand Down