Skip to content

refactor: remove dead code and commented-out tests from escrow and ev…#446

Merged
Xhristin3 merged 4 commits into
rinafcode:mainfrom
Oluwasuyi-Timilehin:chore/366-clean-dead-code
Apr 27, 2026
Merged

refactor: remove dead code and commented-out tests from escrow and ev…#446
Xhristin3 merged 4 commits into
rinafcode:mainfrom
Oluwasuyi-Timilehin:chore/366-clean-dead-code

Conversation

@Oluwasuyi-Timilehin
Copy link
Copy Markdown
Contributor

@Oluwasuyi-Timilehin Oluwasuyi-Timilehin commented Apr 26, 2026

#closes #366

PR Title

chore: remove dead code and unused imports for #366

PR Description

Summary

This PR fully completes issue #366 by performing a focused codebase cleanup in the TeachLink contract:

  • identified and removed dead code,
  • removed unused imports,
  • cleaned obsolete commented-out code,
  • resolved related integration test failures triggered during verification.

The scope is cleanup and stability only; no intentional feature changes were introduced.

Context

During validation, multiple integration tests in test_module_interactions.rs failed with Soroban auth errors (Error(Auth, ExistingValue)).
Root cause was duplicate authorization in role checks (auth required both at caller and inside role-check helper).
This PR includes the minimal fix to preserve behavior while making tests pass reliably.

What Changed

1) Dead Code Removal

  • Removed unused result aliases from contracts/teachlink/src/errors.rs:
    • BridgeResult<T>
    • EscrowResult<T>
    • RewardsResult<T>
    • CommonResult<T>
  • Removed unused helper methods from contracts/teachlink/src/provenance.rs:
    • get_creator
    • get_all_owners
  • Removed associated dead-code suppression attributes in provenance.

2) Unused Imports Cleanup

  • Removed unused imports from contracts/teachlink/src/events.rs:
    • PacketStatus
    • SwapStatus

3) Commented Code Cleanup

  • Removed stale commented event/export sections in contracts/teachlink/src/events.rs.
  • Removed large commented-out repository test block in contracts/teachlink/src/repository/escrow_repository.rs.
  • Removed commented module/test/export leftovers in contracts/teachlink/src/repository/mod.rs.
  • Removed large obsolete block-commented event definitions in contracts/teachlink/src/notification_events.rs.

4) Auth Stability Fix Found During Verification

  • Updated contracts/teachlink/src/access_control.rs:
    • Removed require_auth() from check_role(...) to avoid duplicate auth in the same frame.
    • Added explicit caller.require_auth() in:
      • grant_role(...)
      • revoke_role(...)

This preserves authorization guarantees while preventing Auth ExistingValue panics.

Files Changed

  • contracts/teachlink/src/errors.rs
  • contracts/teachlink/src/events.rs
  • contracts/teachlink/src/repository/escrow_repository.rs
  • contracts/teachlink/src/repository/mod.rs
  • contracts/teachlink/src/provenance.rs
  • contracts/teachlink/src/notification_events.rs
  • contracts/teachlink/src/access_control.rs
  • PR_DESCRIPTION.md (updated for current PR narrative)

Why This Matters

  • Reduces code noise and maintenance overhead.
  • Improves readability and contributor onboarding.
  • Aligns with lint cleanliness goals.
  • Stabilizes cross-module tests by eliminating duplicate auth checks.

Acceptance Criteria Mapping (#366)

  • Identify dead code
  • Remove unused imports
  • Clean up commented code
  • Run linter checks
  • Verify tests for impacted areas

Validation / Test Evidence

Targeted failing tests (now passing)

Previously failing tests:

  • test_chain_specific_pause_isolation
  • test_emergency_pause_and_resume_state_management
  • test_liquidity_pool_affects_bridge_fee
  • test_emergency_state_alongside_packet_operations
  • test_error_propagation_across_modules
  • test_multichain_config_and_packet_workflow
  • test_notification_alongside_bridge_operations
  • test_proposal_vote_then_bridge_completion

After auth fix in access_control, all above were re-run and passed.

Suggested verification commands

git status --short
git diff -- contracts/teachlink/src/errors.rs \
  contracts/teachlink/src/events.rs \
  contracts/teachlink/src/repository/escrow_repository.rs \
  contracts/teachlink/src/repository/mod.rs \
  contracts/teachlink/src/provenance.rs \
  contracts/teachlink/src/notification_events.rs \
  contracts/teachlink/src/access_control.rs
rg "allow\(dead_code\)" contracts/teachlink/src
cargo fmt --all -- --check
cargo clippy --all-targets --all-features -D warnings
cargo test --test test_module_interactions -- --nocapture

Risk Assessment

  • Risk level: Low to Medium
  • Reason: Mostly cleanup-only deletions; one targeted auth-flow correction in RBAC helper behavior.
  • Mitigation: Integration tests covering bridge/emergency/message-passing flows pass after change.

Breaking Changes

None intended.

Notes for Reviewers

  • Focus review on access_control auth semantics (check_role, grant_role, revoke_role) and integration-test behavior around emergency + bridge flows.
  • Cleanup changes are deletion-only and should be straightforward to validate by diff.

#closes

@drips-wave
Copy link
Copy Markdown

drips-wave Bot commented Apr 26, 2026

@Oluwasuyi-Timilehin Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@Xhristin3 Xhristin3 merged commit 54ec62f into rinafcode:main Apr 27, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove dead code and unused imports

2 participants