fix: migrate integration tests to production dlp without unit_test_config #630
Conversation
…nfig (magicblock-labs#602) Replace the precompiled dlp.so binary with a production version compiled without the unit_test_config feature, which hardcoded test validator authorities. Update integration tests and test programs to accept and properly pass custom validator identities instead of relying on hardcoded test values. Fixes magicblock-labs#602 Signed-off-by: AvhiMaz <[email protected]>
WalkthroughPropagate an optional validator identity through delegate-related instruction builders and handlers: add validator-aware instruction APIs, thread an Option into delegate handler/config, update client and tests to pass validator where applicable, deprecate a legacy test utility, and silence deprecated warnings in several test modules. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Test as Test code
participant Client as schedule_commit_context
participant API as schedulecommit::api
participant CPI as flexi-counter CPI
rect rgb(235,245,255)
Test->>Client: request escrow for payer (includes ephem validator)
Client->>API: init_payer_escrow_with_validator(payer, Some(validator))
end
rect rgb(245,255,235)
API->>API: build top_up + delegate instructions (passes validator)
API->>CPI: create_delegate_ix_with_validator(payer, Some(validator))
end
rect rgb(255,245,235)
CPI->>CPI: delegate handler receives validator\nand initializes DelegateConfig { validator }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-10-14T09:56:14.047ZApplied to files:
🧬 Code graph analysis (2)test-integration/test-schedule-intent/tests/test_schedule_intents.rs (1)
test-integration/schedulecommit/client/src/schedule_commit_context.rs (1)
🔇 Additional comments (3)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
test-integration/test-ledger-restore/tests/06_delegated_account.rs (1)
1-2: Deprecation warning suppression.Same deprecation suppression pattern as noted in
test-integration/test-config/tests/auto_airdrop_feepayer.rs. See the comment on that file regarding verification of deprecated APIs.test-integration/test-runner/bin/run_tests.rs (1)
1-2: Deprecation warning suppression.Same deprecation suppression pattern. See the verification request in
test-integration/test-config/tests/auto_airdrop_feepayer.rs.test-integration/test-magicblock-api/tests/test_claim_fees.rs (1)
1-2: Deprecation warning suppression.Same deprecation suppression pattern. See the verification request in
test-integration/test-config/tests/auto_airdrop_feepayer.rs.test-integration/test-task-scheduler/src/lib.rs (1)
1-2: Deprecation warning suppression.Same deprecation suppression pattern. See the verification request in
test-integration/test-config/tests/auto_airdrop_feepayer.rs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
test-integration/schedulecommit/elfs/dlp.sois excluded by!**/*.so
📒 Files selected for processing (12)
test-integration/programs/flexi-counter/src/instruction.rs(1 hunks)test-integration/programs/flexi-counter/src/processor/call_handler.rs(2 hunks)test-integration/programs/schedulecommit/src/api.rs(2 hunks)test-integration/schedulecommit/client/src/schedule_commit_context.rs(2 hunks)test-integration/schedulecommit/test-scenarios/tests/02_commit_and_undelegate.rs(1 hunks)test-integration/test-config/tests/auto_airdrop_feepayer.rs(1 hunks)test-integration/test-ledger-restore/tests/06_delegated_account.rs(1 hunks)test-integration/test-magicblock-api/tests/test_claim_fees.rs(1 hunks)test-integration/test-runner/bin/run_tests.rs(1 hunks)test-integration/test-schedule-intent/tests/test_schedule_intents.rs(3 hunks)test-integration/test-task-scheduler/src/lib.rs(1 hunks)test-integration/test-tools/src/loaded_accounts.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-14T09:56:14.047Z
Learnt from: taco-paco
Repo: magicblock-labs/magicblock-validator PR: 564
File: test-integration/programs/flexi-counter/src/processor/call_handler.rs:122-125
Timestamp: 2025-10-14T09:56:14.047Z
Learning: The file test-integration/programs/flexi-counter/src/processor/call_handler.rs contains a test smart contract used for integration testing, not production code.
Applied to files:
test-integration/programs/flexi-counter/src/instruction.rstest-integration/programs/flexi-counter/src/processor/call_handler.rstest-integration/schedulecommit/test-scenarios/tests/02_commit_and_undelegate.rstest-integration/test-task-scheduler/src/lib.rstest-integration/test-config/tests/auto_airdrop_feepayer.rstest-integration/test-schedule-intent/tests/test_schedule_intents.rs
📚 Learning: 2025-10-21T14:00:54.642Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/websocket/account_subscribe.rs:18-27
Timestamp: 2025-10-21T14:00:54.642Z
Learning: In magicblock-aperture account_subscribe handler (src/requests/websocket/account_subscribe.rs), the RpcAccountInfoConfig fields data_slice, commitment, and min_context_slot are currently ignored—only encoding is applied. This is tracked as technical debt in issue #579: https://github.com/magicblock-labs/magicblock-validator/issues/579
Applied to files:
test-integration/test-ledger-restore/tests/06_delegated_account.rstest-integration/schedulecommit/test-scenarios/tests/02_commit_and_undelegate.rstest-integration/test-config/tests/auto_airdrop_feepayer.rs
📚 Learning: 2025-11-07T13:09:52.253Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: test-kit/src/lib.rs:275-0
Timestamp: 2025-11-07T13:09:52.253Z
Learning: In test-kit, the transaction scheduler in ExecutionTestEnv is not expected to shut down during tests. Therefore, using `.unwrap()` in test helper methods like `schedule_transaction` is acceptable and will not cause issues in the test environment.
Applied to files:
test-integration/schedulecommit/test-scenarios/tests/02_commit_and_undelegate.rs
📚 Learning: 2025-11-07T18:19:48.996Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 614
File: test-integration/programs/schedulecommit/Cargo.toml:19-20
Timestamp: 2025-11-07T18:19:48.996Z
Learning: In Solana programs (crates depending on solana-program), the empty feature flags `custom-heap = []` and `custom-panic = []` are standard declarations required to avoid compiler warnings. These should not be flagged as unused features in Cargo.toml files for Solana program crates.
Applied to files:
test-integration/test-task-scheduler/src/lib.rstest-integration/test-runner/bin/run_tests.rstest-integration/test-magicblock-api/tests/test_claim_fees.rs
🧬 Code graph analysis (2)
test-integration/schedulecommit/client/src/schedule_commit_context.rs (1)
test-integration/programs/schedulecommit/src/api.rs (2)
init_payer_escrow_with_validator(39-63)pda_and_bump(263-267)
test-integration/test-schedule-intent/tests/test_schedule_intents.rs (1)
test-integration/programs/flexi-counter/src/instruction.rs (1)
create_delegate_ix_with_validator(303-308)
🔇 Additional comments (7)
test-integration/schedulecommit/test-scenarios/tests/02_commit_and_undelegate.rs (2)
233-235: Early return on InvalidWritableAccount in simulation is appropriate.The early return when detecting
InvalidWritableAccountin the simulation result correctly handles the transient undelegation state. However, note that string matching on error messages can be fragile if error message formats change.
247-252: LGTM! Proper error handling for undelegated accounts.The structured error matching using
matches!correctly identifies theInvalidWritableAccounterror and returns early, preventing false test failures when accounts are in a transient undelegated state. This is an improvement over the string-based check in the simulation path above.test-integration/programs/flexi-counter/src/processor/call_handler.rs (1)
100-103: LGTM! Validator parameter properly threaded for future use.The addition of the
validatorparameter toprocess_redelegation_call_handlerand its propagation toDelegateConfigaligns with the PR's goal of supporting custom validator identities. While this function is currently dead code due to Solana's reentrancy restrictions, the change prepares the infrastructure for validator-aware delegation when reentrancy becomes possible.Also applies to: 130-133
test-integration/programs/flexi-counter/src/instruction.rs (1)
303-308: Transitional API: validator parameter currently unused.This new function accepts a
validatorparameter but doesn't utilize it—it simply delegates to the existingcreate_delegate_ix. This is a reasonable transitional step to establish the API signature while the underlying implementation is being updated. However, consider:
- Documenting when and how the
validatorparameter will be used- Adding a TODO or tracking issue reference for the full implementation
- Considering whether to mark this as
#[deprecated]once the parameter is actually utilized in a new versionIs there a tracking issue or timeline for when the
validatorparameter will be actively used rather than ignored?test-integration/test-config/tests/auto_airdrop_feepayer.rs (1)
1-2: The deprecated API suppression is justified and documented.The
#![allow(deprecated)]suppresses warnings fromLoadedAccounts::with_delegation_program_test_authority()(line 49), which is marked deprecated in test-integration/test-tools/src/loaded_accounts.rs. The deprecation is well-documented:
- Migration path: Use
new_with_new_validator_authority()instead- Reason: DLP now uses production build without unit_test_config (issue-602)
The current code uses the deprecated method intentionally for backward compatibility with existing test infrastructure. Consider updating to the new API when convenient, though the current approach is acceptable for test code during a transition period.
test-integration/schedulecommit/client/src/schedule_commit_context.rs (1)
8-9: Validator-aware escrow wiring looks correctSwitching to
init_payer_escrow_with_validatorand derivingvalidator_identityfromcommon_ctx.ephem_validator_identity.as_ref().copied()cleanly threads the optional validator without changing behavior when it’sNone. No issues found with the new escrow flow.Also applies to: 198-207
test-integration/programs/schedulecommit/src/api.rs (1)
35-37: New escrow helper cleanly propagates optional validator
init_payer_escrow_with_validatorcentralizes the escrow setup whileinit_payer_escrowremains a backward-compatible wrapper passingNone. Wiring thevalidatorparameter straight intoDelegateArgsis consistent with the DLP API and matches the intent to move off hardcoded test authorities.Also applies to: 39-42, 52-58
test-integration/test-schedule-intent/tests/test_schedule_intents.rs
Outdated
Show resolved
Hide resolved
test-integration/test-schedule-intent/tests/test_schedule_intents.rs
Outdated
Show resolved
Hide resolved
Signed-off-by: AvhiMaz <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test-integration/test-schedule-intent/tests/test_schedule_intents.rs (1)
350-351: Consistent validator-aware delegation usage.The update to
create_delegate_ix_with_validatorpassingctx.ephem_validator_identityis consistent with the new validator-aware delegation flow and matches the pattern established insetup_payer. This keeps all delegation call sites aligned with the PR objective.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
test-integration/test-schedule-intent/tests/test_schedule_intents.rs(3 hunks)test-integration/test-tools/src/loaded_accounts.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-14T09:56:14.047Z
Learnt from: taco-paco
Repo: magicblock-labs/magicblock-validator PR: 564
File: test-integration/programs/flexi-counter/src/processor/call_handler.rs:122-125
Timestamp: 2025-10-14T09:56:14.047Z
Learning: The file test-integration/programs/flexi-counter/src/processor/call_handler.rs contains a test smart contract used for integration testing, not production code.
Applied to files:
test-integration/test-schedule-intent/tests/test_schedule_intents.rs
🧬 Code graph analysis (1)
test-integration/test-schedule-intent/tests/test_schedule_intents.rs (1)
test-integration/programs/flexi-counter/src/instruction.rs (4)
create_add_ix(226-238)create_delegate_ix_with_validator(303-308)create_init_ix(189-202)create_intent_ix(381-422)
🔇 Additional comments (2)
test-integration/test-schedule-intent/tests/test_schedule_intents.rs (2)
7-8: LGTM: Import updated for validator-aware delegation.The import change from
create_delegate_ixtocreate_delegate_ix_with_validatorcorrectly reflects the new validator-aware delegation API, aligning with the PR objective to pass explicit validator identities instead of relying on hardcoded test values.
291-311: Excellent fix: Index consistency issue resolved and validator identity properly propagated.The previous critical issue regarding index mismatch has been addressed. The code now consistently uses index
1for:
top_up_ephemeral_balance(line 296)delegate_ephemeral_balance(line 302)- PDA assertion (line 314)
The explicit validator identity propagation via
ctx.ephem_validator_identity(line 306) correctly implements the PR objective to move away from hardcoded test validator authorities. The two-step delegation (top-up followed by delegate) is sent atomically in a single transaction (line 310), preventing any intermediate failure states.
Signed-off-by: AvhiMaz <[email protected]>
Signed-off-by: AvhiMaz <[email protected]>
1 participant
Replace the precompiled dlp.so binary with a production version compiled without
the unit_test_config feature, which hardcoded test validator authorities. Update
integration tests and test programs to accept and properly pass custom validator
identities instead of relying on hardcoded test values.
Fixes #602
Summary by CodeRabbit
New Features
Deprecations
Bug Fixes
Chores