Skip to content

fix: Liquidator MCR target, API key header, loop position re-fetch#376

Merged
vitaliybezz merged 2 commits intodevfrom
fix/liquidator-mcr-target
Feb 27, 2026
Merged

fix: Liquidator MCR target, API key header, loop position re-fetch#376
vitaliybezz merged 2 commits intodevfrom
fix/liquidator-mcr-target

Conversation

@vitaliybezz
Copy link
Contributor

@vitaliybezz vitaliybezz commented Feb 27, 2026

This change is Reviewable

@coderabbitai
Copy link

coderabbitai bot commented Feb 27, 2026

📝 Walkthrough

Summary

This PR addresses three distinct concerns in the liquidator service:

1. Environment Variable Standardization

Renamed generic environment variables to NEAR-specific naming:

  • NETWORKNEAR_NETWORK
  • RPC_URLNEAR_RPC_URL
  • Added NEAR_API_KEY support

Updates applied across configuration files (.env.example), shell scripts, config.rs, and service.rs.

2. RPC API Key Header Support

Added optional API key authentication for RPC calls:

  • New ServiceConfig.near_api_key field (bound to NEAR_API_KEY env var)
  • LiquidatorService now constructs and attaches X-API-Key header when near_api_key is provided
  • Uses near_jsonrpc_client::auth::ApiKey with panic on invalid key format

3. Liquidation Logic Refinements

  • MCR threshold change: Replaced borrow_mcr_liquidation with borrow_mcr_maintenance when computing liquidatable collateral (critical threshold change affecting liquidation eligibility)
  • Loop position re-fetch: Added position state refresh after each liquidation iteration
    • Fetches updated position via new MarketScanner::get_borrow_position() method
    • Terminates with Liquidated if position no longer exists
    • Logs warnings on fetch failures and continues looping

Critical Review Points

  1. MCR Threshold Change: The switch from borrow_mcr_liquidation to borrow_mcr_maintenance has business logic implications—verify this is the intended liquidation trigger point and won't cause cascading liquidations at unintended thresholds.

  2. API Key Security: Confirm the panic on invalid API key format is appropriate and won't cause production outages during configuration issues. Verify header construction properly escapes/validates the key.

  3. Position Re-fetch Logic: Verify the decision to continue looping after fetch failures (rather than terminating) is correct. Confirm the loop termination condition on missing positions doesn't cause unexpected early exits.

  4. Field Signature Change: MarketConfiguration.borrow_mcr_liquidationborrow_mcr_maintenance is a public API change—verify all consumers are updated and this is the canonical source.

Walkthrough

The PR renames generic env/CLI vars to NEAR-specific names (NEAR_NETWORK, NEAR_RPC_URL, NEAR_API_KEY), adds optional RPC API-key support, introduces position re-fetching in the liquidation loop, and adds a MarketScanner method to fetch a single borrow position.

Changes

Cohort / File(s) Summary
Env example
service/liquidator/.env.example
Renamed env keys to NEAR-prefixed names and adjusted placeholders.
Config & Service
service/liquidator/src/config.rs, service/liquidator/src/service.rs
Args/env bindings changed to NEAR_NETWORK, NEAR_RPC_URL, NEAR_API_KEY; rpc_url replaced by near_rpc_url and near_api_key added; JsonRpcClient optionally configured with an X-API-Key header.
Scripts
service/liquidator/scripts/intent-swap.sh, service/liquidator/scripts/run-mainnet.sh, service/liquidator/scripts/run-testnet.sh
Shell scripts updated to read NEAR_RPC_URL/NEAR_API_KEY and pass --near-rpc-url/--near-api-key flags to the liquidator invocation.
Liquidator loop
service/liquidator/src/liquidator.rs
Introduced mutable position binding and re-fetch after each liquidation iteration; switched liquidation threshold reference to borrow_mcr_maintenance; added logging and flow changes to terminate when position disappears.
Scanner
service/liquidator/src/scanner.rs
Added pub async fn get_borrow_position(&self, account_id: &AccountId) -> Result<Option<BorrowPosition>, RpcError> to fetch a single borrow position via RPC.
Minor
service/liquidator/src/main.rs
Removed a single-line comment; no behavioral change.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through code to rename and tweak,
NEAR keys now flourish, no longer meek.
I fetch each borrow, then chase the loop—
API keys snug, and logs that whoop! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description is minimal and only contains a Reviewable integration snippet with no meaningful content about the changeset. Provide a detailed description explaining the purpose and impact of the changes, including context about the MCR target fix, API key authentication, and position re-fetch improvements.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the three main changes: MCR target fix, API key header support, and loop position re-fetch logic in the liquidator.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/liquidator-mcr-target

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@service/liquidator/scripts/intent-swap.sh`:
- Line 53: The NEAR_RPC_URL variable in intent-swap.sh is declared but unused;
either remove NEAR_RPC_URL or update all near CLI invocations (e.g., the near
contract call-function / near call commands in this script) to pass the custom
RPC endpoint by adding the --rpc-url flag and referencing NEAR_RPC_URL alongside
the existing NETWORK usage so the CLI actually uses the custom endpoint.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a37b43 and c26b2fb.

📒 Files selected for processing (9)
  • service/liquidator/.env.example
  • service/liquidator/scripts/intent-swap.sh
  • service/liquidator/scripts/run-mainnet.sh
  • service/liquidator/scripts/run-testnet.sh
  • service/liquidator/src/config.rs
  • service/liquidator/src/liquidator.rs
  • service/liquidator/src/main.rs
  • service/liquidator/src/scanner.rs
  • service/liquidator/src/service.rs
💤 Files with no reviewable changes (1)
  • service/liquidator/src/main.rs

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
service/liquidator/src/config.rs (1)

430-458: 🧹 Nitpick | 🔵 Trivial

Consider adding test coverage for near_api_key.

The test verifies near_rpc_url propagation but doesn't test near_api_key. Adding an assertion ensures the API key is correctly passed through to ServiceConfig.

💡 Proposed test enhancement
     fn test_build_config() {
         let mut args = create_test_args();
         args.near_rpc_url = Some("https://custom.rpc.url".to_string());
+        args.near_api_key = Some("test_api_key".to_string());
         args.transaction_timeout = 90;
         // ... existing setup ...

         let config = args.build_config();
         // ... existing assertions ...
         assert_eq!(
             config.near_rpc_url,
             Some("https://custom.rpc.url".to_string())
         );
+        assert_eq!(
+            config.near_api_key,
+            Some("test_api_key".to_string())
+        );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@service/liquidator/src/config.rs` around lines 430 - 458, The test function
test_build_config currently checks near_rpc_url but omits near_api_key; update
the test (in test_build_config using create_test_args / args.build_config) to
set args.near_api_key = Some("<expected_key>".to_string()) and add an assertion
that config.near_api_key equals that Some value so the ServiceConfig propagation
is verified.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@service/liquidator/src/config.rs`:
- Around line 430-458: The test function test_build_config currently checks
near_rpc_url but omits near_api_key; update the test (in test_build_config using
create_test_args / args.build_config) to set args.near_api_key =
Some("<expected_key>".to_string()) and add an assertion that config.near_api_key
equals that Some value so the ServiceConfig propagation is verified.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c26b2fb and d1e3b16.

📒 Files selected for processing (2)
  • service/liquidator/src/config.rs
  • service/liquidator/src/liquidator.rs

@codecov
Copy link

codecov bot commented Feb 27, 2026

Codecov Report

❌ Patch coverage is 42.85714% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
service/liquidator/src/service.rs 0.00% 11 Missing ⚠️
service/liquidator/src/config.rs 90.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

let liquidatable_collateral = position.liquidatable_collateral(
&price_pair,
self.market_config.borrow_mcr_liquidation,
self.market_config.borrow_mcr_maintenance,
Copy link
Collaborator

Choose a reason for hiding this comment

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

cool. this is the key change

@vitaliybezz vitaliybezz merged commit 91249ed into dev Feb 27, 2026
14 checks passed
@vitaliybezz vitaliybezz deleted the fix/liquidator-mcr-target branch February 27, 2026 23:34
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.

2 participants