fix: lease close reason

[vertex451]

Description

Closes: akash-network/support#403

1. Made sure the reason is stored onchain.
2. Fixed bug with hardcoded LeaseCloseReason.
3. Tests, of course.

Manual testing results

Insufficient funds scenario:

akash q market lease get --dseq $DSEQ --owner akash1fjdy5x8v936l34pxgdn3e0cg30k22n7je6hph9 --provider akash1q8t0g9yalktyla6xrmjz4d6j0pyz4etnk2z9rz
escrow_payment:
  id:
    aid:
      scope: deployment
      xid: akash1fjdy5x8v936l34pxgdn3e0cg30k22n7je6hph9/12
    xid: 1/1/akash1q8t0g9yalktyla6xrmjz4d6j0pyz4etnk2z9rz
  state:
    balance:
      amount: "0.000000000000000000"
      denom: uakt
    owner: akash1q8t0g9yalktyla6xrmjz4d6j0pyz4etnk2z9rz
    rate:
      amount: "1031634.370000000000000000"
      denom: uakt
    state: overdrawn
    unsettled:
      amount: "124327758.000000000000000000"
      denom: uakt
    withdrawn:
      amount: "500000"
      denom: uakt
lease:
  closed_on: "7065"
  created_at: "6944"
  id:
    bseq: 0
    dseq: "12"
    gseq: 1
    oseq: 1
    owner: akash1fjdy5x8v936l34pxgdn3e0cg30k22n7je6hph9
    provider: akash1q8t0g9yalktyla6xrmjz4d6j0pyz4etnk2z9rz
  price:
    amount: "1031634.370000000000000000"
    denom: uakt
  reason: lease_closed_reason_insufficient_funds
  state: insufficient_funds

Unstable deployment scenario:

akash q market lease get --dseq $DSEQ --owner akash1fjdy5x8v936l34pxgdn3e0cg30k22n7je6hph9 --provider akash1q8t0g9yalktyla6xrmjz4d6j0pyz4etnk2z9rz
escrow_payment:
  id:
    aid:
      scope: deployment
      xid: akash1fjdy5x8v936l34pxgdn3e0cg30k22n7je6hph9/13
    xid: 1/1/akash1q8t0g9yalktyla6xrmjz4d6j0pyz4etnk2z9rz
  state:
    balance:
      amount: "0.000000000000000000"
      denom: uakt
    owner: akash1q8t0g9yalktyla6xrmjz4d6j0pyz4etnk2z9rz
    rate:
      amount: "13.679111000000000000"
      denom: uakt
    state: closed
    unsettled:
      amount: "0.000000000000000000"
      denom: uakt
    withdrawn:
      amount: "1272"
      denom: uakt
lease:
  closed_on: "7985"
  created_at: "7892"
  id:
    bseq: 0
    dseq: "13"
    gseq: 1
    oseq: 1
    owner: akash1fjdy5x8v936l34pxgdn3e0cg30k22n7je6hph9
    provider: akash1q8t0g9yalktyla6xrmjz4d6j0pyz4etnk2z9rz
  price:
    amount: "13.679111000000000000"
    denom: uakt
  reason: lease_closed_reason_unstable
  state: closed

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.

I have...

• included the correct type prefix in the PR title
• added ! to the type prefix if API or client breaking change
• targeted the correct branch (see PR Targeting)
• provided a link to the relevant issue or specification
• included the necessary unit and integration tests
• added a changelog entry to CHANGELOG.md
• included comments for documenting Go code
• updated the relevant documentation or specification
• reviewed "Files changed" and left comments if necessary
• confirmed all CI checks have passed

[coderabbitai]

Walkthrough

The changes extend the OnGroupClosed interface signature to accept a group state parameter, enabling the market keeper to determine appropriate lease closure reasons based on whether the escrow account is overdrawn. This propagation occurs through the deployment handler, market hooks, and keeper implementation, allowing insufficient fund closures to be tracked separately from owner-initiated closures.

Changes

Cohort / File(s)	Summary
Interface Signatures x/deployment/handler/keepers.go, x/market/hooks/external.go	OnGroupClosed method signature updated to include state parameter of type v1beta4.Group_State, requiring all implementations to adapt.
Deployment Handler Call Sites x/deployment/handler/server.go	Updated OnGroupClosed invocations in CloseGroup and PauseGroup to pass group state: types.GroupClosed or types.GroupPaused.
Market Hooks Implementation x/market/hooks/hooks.go	OnEscrowAccountClosed now computes group state as GroupClosed by default or GroupInsufficientFunds if escrow is overdrawn, then passes it to OnGroupClosed.
Market Keeper Implementation x/market/keeper/keeper.go	OnGroupClosed now uses group state parameter to conditionally set lease close state to LeaseInsufficientFunds (with reason flag) when group state is GroupInsufficientFunds, and attempts payment closure for each closed lease.
Test Updates x/market/handler/handler_test.go, x/market/keeper/keeper_test.go	Handler tests updated for lease state expectations; keeper tests refactored to table-driven approach with multiple state/reason scenarios, expanded idempotency coverage, and comprehensive group closure validation. Import alias changed to mv1 for market types.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A group's farewell now carries context,
Whether closed by owner or funds that checked,
State flows gently through the market chain,
Leases now remember loss and gain,
Insufficient funds no longer stay unnamed! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: lease close reason' directly relates to the main objective of fixing the hardcoded lease close reason issue and ensuring correct reasons are stored on-chain.
Description check	✅ Passed	The description provides a clear explanation of the fix, links to the related issue #403, outlines the three key changes, and includes manual testing results demonstrating the fix working correctly for both insufficient funds and unstable deployment scenarios.
Linked Issues check	✅ Passed	The code changes fully implement the requirements from issue #403: the lease close reason is now dynamically set based on group state (insufficient funds vs. owner closure), passed through the MarketKeeper.OnGroupClosed chain, and stored on-chain, fixing the hardcoded LeaseClosedReasonInvalid issue.
Out of Scope Changes check	✅ Passed	All changes are within scope: updated OnGroupClosed signature across interfaces, adjusted call sites, refactored tests to verify the new state parameter behavior, and fixed lease close reason logic - all directly supporting the issue #403 objectives.

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch artem/fix-lease-close-reason

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)

x/deployment/handler/server.go (1)

156-162: Don’t ignore OnGroupClosed errors.

If OnGroupClosed fails, the deployment group is closed but market state can remain partially open. Propagating the error keeps state transitions atomic.

🔧 Proposed fix

-	_ = ms.market.OnGroupClosed(ctx, group.ID, types.GroupClosed)
+	if err := ms.market.OnGroupClosed(ctx, group.ID, types.GroupClosed); err != nil {
+		return nil, err
+	}

-	_ = ms.market.OnGroupClosed(ctx, group.ID, types.GroupPaused)
+	if err := ms.market.OnGroupClosed(ctx, group.ID, types.GroupPaused); err != nil {
+		return nil, err
+	}

Also applies to: 180-186

[troian]

keep it v1 or rename to mv1

[troian]

lets not update changelog, we plan to auto update it using changelog generators

[vertex451]

Okay, reverted.

[troian]

any reason why test was removed

[vertex451]

This is not the test, this is helper that creates a deployment and it is unused.