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
2 changes: 1 addition & 1 deletion .github/workflows/ci-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:
- uses: ./magicblock-validator/.github/actions/setup-build-env
with:
build_cache_key_name: "magicblock-validator-ci-lint-v002"
rust_toolchain_release: "1.84.1"
rust_toolchain_release: "1.91.1"
github_access_token: ${{ secrets.GH_PERSONAL_ACCESS_TOKEN }}
github_token: ${{ secrets.GITHUB_TOKEN }}

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci-test-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
- uses: ./magicblock-validator/.github/actions/setup-build-env
with:
build_cache_key_name: "magicblock-validator-ci-test-integration-${{ github.ref_name }}-${{ hashFiles('magicblock-validator/Cargo.lock') }}"
rust_toolchain_release: "1.84.1"
rust_toolchain_release: "1.91.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Inconsistent toolchain versions across CI jobs.

The build job uses 1.91.1 (line 27), but the run_integration_tests job still uses 1.84.1 (line 68). This creates a mismatch: code compiled with one toolchain is tested with another, which can mask toolchain-specific bugs or inconsistencies.

Update line 68 to 1.91.1 to maintain consistency across all CI jobs.

Apply this diff to fix the inconsistency:

      - uses: ./magicblock-validator/.github/actions/setup-build-env
         with:
           build_cache_key_name: "magicblock-validator-ci-test-integration-${{ github.ref_name }}-${{ hashFiles('magicblock-validator/Cargo.lock') }}"
-          rust_toolchain_release: "1.84.1"
+          rust_toolchain_release: "1.91.1"
           github_access_token: ${{ secrets.GH_PERSONAL_ACCESS_TOKEN }}
           github_token: ${{ secrets.GITHUB_TOKEN }}

Also applies to: 68-68

🤖 Prompt for AI Agents
.github/workflows/ci-test-integration.yml lines 27 and 68: the Rust toolchain
versions are inconsistent across CI jobs (line 27 shows 1.91.1 while line 68
uses 1.84.1); update the rust_toolchain_release value at line 68 from 1.84.1 to
1.91.1 so both build and run_integration_tests jobs use the same 1.91.1
toolchain.

github_access_token: ${{ secrets.GH_PERSONAL_ACCESS_TOKEN }}
github_token: ${{ secrets.GITHUB_TOKEN }}

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci-test-unit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:
- uses: ./magicblock-validator/.github/actions/setup-build-env
with:
build_cache_key_name: "magicblock-validator-ci-test-unit-v000"
rust_toolchain_release: "1.84.1"
rust_toolchain_release: "1.91.1"
github_access_token: ${{ secrets.GH_PERSONAL_ACCESS_TOKEN }}
github_token: ${{ secrets.GITHUB_TOKEN }}

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/deploy-testnet-by-pr-comment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -159,4 +159,4 @@ jobs:
repo: context.repo.repo,
issue_number: context.payload.issue.number,
body: `❌ Failed to trigger deploy for branch \`${{ steps.get_branch.outputs.branch }}\`. Make sure you have permission or use the manual deploy link in the PR comment.`
});
});
2 changes: 1 addition & 1 deletion magicblock-accounts-db/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ impl AccountsDbIndex {
/// Obtain a wrapped cursor to query account offsets repeatedly
pub(crate) fn offset_finder(
&self,
) -> AccountsDbResult<AccountOffsetFinder> {
) -> AccountsDbResult<AccountOffsetFinder<'_>> {
let txn = self.env.begin_ro_txn()?;
AccountOffsetFinder::new(&self.accounts, txn)
}
Expand Down
4 changes: 2 additions & 2 deletions magicblock-accounts-db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ impl AccountsDb {
&self,
program: &Pubkey,
filter: F,
) -> AccountsDbResult<AccountsScanner<F>>
) -> AccountsDbResult<AccountsScanner<'_, F>>
where
F: Fn(&AccountSharedData) -> bool + 'static,
{
Expand Down Expand Up @@ -246,7 +246,7 @@ impl AccountsDb {
pub fn set_slot(self: &Arc<Self>, slot: u64) {
self.storage.set_slot(slot);

if 0 != slot % self.snapshot_frequency {
if !slot.is_multiple_of(self.snapshot_frequency) {
return;
}
let this = self.clone();
Expand Down
8 changes: 7 additions & 1 deletion magicblock-accounts/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,13 @@ pub enum ScheduledCommitsProcessorError {
#[error("RecvError: {0}")]
RecvError(#[from] RecvError),
#[error("CommittorSerivceError")]
CommittorSerivceError(#[from] CommittorServiceError),
CommittorSerivceError(Box<CommittorServiceError>),
}

impl From<CommittorServiceError> for ScheduledCommitsProcessorError {
fn from(e: CommittorServiceError) -> Self {
Self::CommittorSerivceError(Box::new(e))
}
}

pub type ScheduledCommitsProcessorResult<
Expand Down
52 changes: 46 additions & 6 deletions magicblock-api/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,16 @@ pub enum ApiError {
RpcError(#[from] magicblock_aperture::error::RpcError),

#[error("Accounts error: {0}")]
AccountsError(#[from] magicblock_accounts::errors::AccountsError),
AccountsError(Box<magicblock_accounts::errors::AccountsError>),

#[error("AccountCloner error: {0}")]
AccountClonerError(#[from] magicblock_account_cloner::AccountClonerError),
AccountClonerError(Box<magicblock_account_cloner::AccountClonerError>),

#[error("Ledger error: {0}")]
LedgerError(#[from] magicblock_ledger::errors::LedgerError),
LedgerError(Box<magicblock_ledger::errors::LedgerError>),

#[error("Chainlink error: {0}")]
ChainlinkError(#[from] magicblock_chainlink::errors::ChainlinkError),
ChainlinkError(Box<magicblock_chainlink::errors::ChainlinkError>),

#[error("Failed to obtain balance for validator '{0}' from chain. ({1})")]
FailedToObtainValidatorOnChainBalance(Pubkey, String),
Expand All @@ -32,7 +32,7 @@ pub enum ApiError {

#[error("CommittorServiceError")]
CommittorServiceError(
#[from] magicblock_committor_service::error::CommittorServiceError,
Box<magicblock_committor_service::error::CommittorServiceError>,
),

#[error("Failed to load programs into bank: {0}")]
Expand Down Expand Up @@ -91,11 +91,51 @@ pub enum ApiError {

#[error("TaskSchedulerServiceError")]
TaskSchedulerServiceError(
#[from] magicblock_task_scheduler::TaskSchedulerError,
Box<magicblock_task_scheduler::TaskSchedulerError>,
),

#[error("Failed to sanitize transaction: {0}")]
FailedToSanitizeTransaction(
#[from] solana_sdk::transaction::TransactionError,
),
}

impl From<magicblock_accounts::errors::AccountsError> for ApiError {
fn from(e: magicblock_accounts::errors::AccountsError) -> Self {
Self::AccountsError(Box::new(e))
}
}

impl From<magicblock_account_cloner::AccountClonerError> for ApiError {
fn from(e: magicblock_account_cloner::AccountClonerError) -> Self {
Self::AccountClonerError(Box::new(e))
}
}

impl From<magicblock_ledger::errors::LedgerError> for ApiError {
fn from(e: magicblock_ledger::errors::LedgerError) -> Self {
Self::LedgerError(Box::new(e))
}
}

impl From<magicblock_chainlink::errors::ChainlinkError> for ApiError {
fn from(e: magicblock_chainlink::errors::ChainlinkError) -> Self {
Self::ChainlinkError(Box::new(e))
}
}

impl From<magicblock_committor_service::error::CommittorServiceError>
for ApiError
{
fn from(
e: magicblock_committor_service::error::CommittorServiceError,
) -> Self {
Self::CommittorServiceError(Box::new(e))
}
}

impl From<magicblock_task_scheduler::TaskSchedulerError> for ApiError {
fn from(e: magicblock_task_scheduler::TaskSchedulerError) -> Self {
Self::TaskSchedulerServiceError(Box::new(e))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use solana_pubkey::Pubkey;
/// - `pubkey`: the account pubkey
/// - `is_delegated_on_chain`: whether the account is currently delegated to us on chain
/// - `remote_slot_in_bank`: the chain slot at which we last fetched and cloned state
/// of the account in our bank
/// of the account in our bank
/// - `delegation_record`: the delegation record associated with the account in our bank, if found
/// - `validator_auth`: the validator authority pubkey
/// - returns `true` if the account is still undelegating, `false` otherwise.
Expand Down
10 changes: 9 additions & 1 deletion magicblock-chainlink/src/remote_account_provider/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ pub type RemoteAccountProviderResult<T> =
pub enum RemoteAccountProviderError {
#[error("Pubsub client error: {0}")]
PubsubClientError(
#[from] solana_pubsub_client::pubsub_client::PubsubClientError,
Box<solana_pubsub_client::pubsub_client::PubsubClientError>,
),

#[error(transparent)]
Expand Down Expand Up @@ -88,3 +88,11 @@ pub enum RemoteAccountProviderError {
)]
LoaderV4StateDeserializationFailed(Pubkey, String),
}

impl From<solana_pubsub_client::pubsub_client::PubsubClientError>
for RemoteAccountProviderError
{
fn from(e: solana_pubsub_client::pubsub_client::PubsubClientError) -> Self {
Self::PubsubClientError(Box::new(e))
}
}
2 changes: 2 additions & 0 deletions magicblock-committor-program/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(clippy::manual_is_multiple_of)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed because build-sbf does not have the feature enabled? Would be nice to avoid it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, unfortunately solana uses older toolchain to build, but clippy runs with newer one, we have to silence it until solana upgrades the toolchain to 1.87 or higher.


Comment on lines +1 to +2
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Document the toolchain constraint with an inline comment.

The crate-level allow is justified given Solana's older toolchain constraint (as explained in the discussion). However, add an inline comment explaining why this exists and when it can be removed to help future maintainers.

Apply this diff to add documentation:

+// TODO: Remove once Solana upgrades to toolchain 1.87+
+// Needed because Solana build uses older toolchain but clippy runs with 1.91.1
 #![allow(clippy::manual_is_multiple_of)]
📝 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
#![allow(clippy::manual_is_multiple_of)]
// TODO: Remove once Solana upgrades to toolchain 1.87+
// Needed because Solana build uses older toolchain but clippy runs with 1.91.1
#![allow(clippy::manual_is_multiple_of)]
🤖 Prompt for AI Agents
In magicblock-committor-program/src/lib.rs around lines 1 to 2, add an inline
comment next to the crate-level allow attribute explaining that
clippy::manual_is_multiple_of is allowed because the crate must maintain
compatibility with an older Solana toolchain that lacks the newer is_multiple_of
intrinsic; state when it can be removed (e.g., after upgrading the Solana
toolchain to a version that provides the intrinsic or when the minimum supported
Rust/toolchain is bumped), and include a short reference to the issue/discussion
or PR that justifies the exception.

use solana_pubkey::declare_id;
pub mod consts;
pub mod error;
Expand Down
2 changes: 1 addition & 1 deletion magicblock-committor-program/src/state/changeset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ impl CommitableAccount {
1
} else {
let count = len / chunk_size as usize;
if len % chunk_size as usize > 0 {
if len % chunk_size as usize != 0 {
count + 1
} else {
count
Expand Down
2 changes: 1 addition & 1 deletion magicblock-committor-program/src/state/chunks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ mod test {
use super::*;

impl Chunks {
pub(super) fn iter(&self) -> ChunksIter {
pub(super) fn iter(&self) -> ChunksIter<'_> {
ChunksIter {
chunks: self,
idx: 0,
Expand Down
3 changes: 1 addition & 2 deletions magicblock-committor-service/src/compute_budget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ impl BufferWriteChunkBudget {
.unwrap_or(u32::MAX as usize),
)
.unwrap_or(u32::MAX)
.checked_add(self.base_budget)
.unwrap_or(u32::MAX)
.saturating_add(self.base_budget)
}

pub fn instructions(&self, bytes_count: usize) -> Vec<Instruction> {
Expand Down
4 changes: 2 additions & 2 deletions magicblock-committor-service/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ pub enum CommittorServiceError {

#[error("Failed to send init changeset account: {0} ({0:?})")]
FailedToSendInitChangesetAccount(
solana_rpc_client_api::client_error::Error,
Box<solana_rpc_client_api::client_error::Error>,
),

#[error("Failed to confirm init changeset account: {0} ({0:?})")]
FailedToConfirmInitChangesetAccount(
solana_rpc_client_api::client_error::Error,
Box<solana_rpc_client_api::client_error::Error>,
),
#[error("Init transaction '{0}' was not confirmed")]
InitChangesetAccountNotConfirmed(String),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ where
#[cfg(test)]
mod tests {
use std::{
collections::{HashMap, HashSet},
collections::HashSet,
sync::{
atomic::{AtomicUsize, Ordering},
Arc,
Expand All @@ -386,9 +386,6 @@ mod tests {
},
intent_executor::{
error::{IntentExecutorError as ExecutorError, InternalError},
task_info_fetcher::{
ResetType, TaskInfoFetcher, TaskInfoFetcherResult,
},
IntentExecutionResult,
},
persist::IntentPersisterImpl,
Expand Down Expand Up @@ -821,29 +818,4 @@ mod tests {
Ok(())
}
}

#[derive(Clone)]
pub struct MockInfoFetcher;
#[async_trait]
impl TaskInfoFetcher for MockInfoFetcher {
async fn fetch_next_commit_ids(
&self,
pubkeys: &[Pubkey],
) -> TaskInfoFetcherResult<HashMap<Pubkey, u64>> {
Ok(pubkeys.iter().map(|&k| (k, 1)).collect())
}

async fn fetch_rent_reimbursements(
&self,
pubkeys: &[Pubkey],
) -> TaskInfoFetcherResult<Vec<Pubkey>> {
Ok(pubkeys.iter().map(|_| Pubkey::new_unique()).collect())
}

fn peek_commit_id(&self, _pubkey: &Pubkey) -> Option<u64> {
None
}

fn reset(&self, _: ResetType) {}
}
}
10 changes: 8 additions & 2 deletions magicblock-committor-service/src/intent_executor/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,13 @@ pub enum InternalError {
#[error("SignerError: {0}")]
SignerError(#[from] SignerError),
#[error("MagicBlockRpcClientError: {0}")]
MagicBlockRpcClientError(#[from] MagicBlockRpcClientError),
MagicBlockRpcClientError(Box<MagicBlockRpcClientError>),
}

impl From<MagicBlockRpcClientError> for InternalError {
fn from(e: MagicBlockRpcClientError) -> Self {
Self::MagicBlockRpcClientError(Box::new(e))
}
}

impl InternalError {
Expand Down Expand Up @@ -144,7 +150,7 @@ pub enum TransactionStrategyExecutionError {

impl From<MagicBlockRpcClientError> for TransactionStrategyExecutionError {
fn from(value: MagicBlockRpcClientError) -> Self {
Self::InternalError(InternalError::MagicBlockRpcClientError(value))
Self::InternalError(InternalError::from(value))
}
}

Expand Down
2 changes: 1 addition & 1 deletion magicblock-committor-service/src/intent_executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ where
InternalError::MagicBlockRpcClientError(err) => {
map_magicblock_client_error(
&self.transaction_error_mapper,
err,
*err,
)
}
err => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ use magicblock_rpc_client::{MagicBlockRpcClientError, MagicblockRpcClient};
use solana_pubkey::Pubkey;
use solana_sdk::signature::Signature;

const NUM_FETCH_RETRIES: NonZeroUsize =
unsafe { NonZeroUsize::new_unchecked(5) };
const NUM_FETCH_RETRIES: NonZeroUsize = NonZeroUsize::new(5).unwrap();
const MUTEX_POISONED_MSG: &str = "CacheTaskInfoFetcher mutex poisoned!";

#[async_trait]
Expand Down Expand Up @@ -51,8 +50,7 @@ pub struct CacheTaskInfoFetcher {

impl CacheTaskInfoFetcher {
pub fn new(rpc_client: MagicblockRpcClient) -> Self {
const CACHE_SIZE: NonZeroUsize =
unsafe { NonZeroUsize::new_unchecked(1000) };
const CACHE_SIZE: NonZeroUsize = NonZeroUsize::new(1000).unwrap();

Self {
rpc_client,
Expand Down Expand Up @@ -274,7 +272,13 @@ pub enum TaskInfoFetcherError {
#[error("InvalidAccountDataError for: {0}")]
InvalidAccountDataError(Pubkey),
#[error("MagicBlockRpcClientError: {0}")]
MagicBlockRpcClientError(#[from] MagicBlockRpcClientError),
MagicBlockRpcClientError(Box<MagicBlockRpcClientError>),
}

impl From<MagicBlockRpcClientError> for TaskInfoFetcherError {
fn from(e: MagicBlockRpcClientError) -> Self {
Self::MagicBlockRpcClientError(Box::new(e))
}
}

impl TaskInfoFetcherError {
Expand Down
12 changes: 8 additions & 4 deletions magicblock-committor-service/src/persist/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,8 @@ mod tests {
fn test_set_commit_id() {
let (mut db, _file) = setup_test_db();
let row = create_test_row(1, 0);
db.insert_commit_status_rows(&[row.clone()]).unwrap();
db.insert_commit_status_rows(std::slice::from_ref(&row))
.unwrap();

// Update commit_id
db.set_commit_id(1, &row.pubkey, 100).unwrap();
Expand All @@ -867,7 +868,8 @@ mod tests {
fn test_update_status_by_message() {
let (mut db, _file) = setup_test_db();
let row = create_test_row(1, 0);
db.insert_commit_status_rows(&[row.clone()]).unwrap();
db.insert_commit_status_rows(std::slice::from_ref(&row))
.unwrap();

let new_status = CommitStatus::Pending;
db.update_status_by_message(1, &row.pubkey, &new_status)
Expand All @@ -881,7 +883,8 @@ mod tests {
fn test_update_status_by_commit() {
let (mut db, _file) = setup_test_db();
let row = create_test_row(1, 100); // Set commit_id to 100
db.insert_commit_status_rows(&[row.clone()]).unwrap();
db.insert_commit_status_rows(std::slice::from_ref(&row))
.unwrap();

let new_status = CommitStatus::Succeeded(CommitStatusSignatures {
commit_stage_signature: Signature::new_unique(),
Expand All @@ -898,7 +901,8 @@ mod tests {
fn test_set_commit_strategy() {
let (mut db, _file) = setup_test_db();
let row = create_test_row(1, 100);
db.insert_commit_status_rows(&[row.clone()]).unwrap();
db.insert_commit_status_rows(std::slice::from_ref(&row))
.unwrap();

let new_strategy = CommitStrategy::FromBuffer;
db.set_commit_strategy(100, &row.pubkey, new_strategy)
Expand Down
Loading