Skip to content

feat(minter): let deposit_sol yield to a deposit queued by a scan during its balance read - #286

Merged
gregorydemay merged 0 commit into
DEFI-2780-10-watchlist-snapshotfrom
DEFI-2780-11-deposit-sol-yields-to-scan
Oct 15, 2026
Merged

gregorydemay merged 0 commit into
DEFI-2780-10-watchlist-snapshotfrom
DEFI-2780-11-deposit-sol-yields-to-scan

Conversation

@gregorydemay

@gregorydemay gregorydemay commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

deposit_sol and the scan of the automatic deposit flow of #272 may read the balance of the same address concurrently. deposit_sol now records the latest deposit of the account before its read and, if a scan queued a deposit in the meantime, returns that deposit and charges only the read, so that the same balance is never queued twice. Covered by two unit tests interleaving the two paths.

🤖 Generated with Claude Code

@gregorydemay
gregorydemay added this pull request to stack #288 October 14, 2026 08:50
Copilot AI balanced review requested due to automatic review settings October 14, 2026 09:04
@gregorydemay
gregorydemay force-pushed the DEFI-2780-11-deposit-sol-yields-to-scan branch from d0e451a to 57bcefc Compare October 14, 2026 09:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

A formatting error will fail the repository’s mandatory formatting check.

1 open finding
What changed in this PR

Prevents deposit_sol from duplicating deposits queued concurrently by automatic scans.

Changes:

  • Detects deposits queued during balance reads.
  • Charges only RPC costs when yielding to a scan.
  • Adds concurrency tests and realistic mocked-call suspension.
File Description
minter/​src/​deposit/​sweep/​mod.rs Handles concurrently queued deposits.
minter/​src/​deposit/​sweep/​tests.rs Tests successful and rejected read races.
minter/​src/​test_fixtures/​runtime.rs Simulates inter-canister call suspension.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.


#[tokio::test]
async fn should_return_the_deposit_a_scan_queued_during_a_rejected_balance_read_and_charge_the_read_only()
{
Copilot AI balanced review requested due to automatic review settings October 14, 2026 09:18
@gregorydemay
gregorydemay force-pushed the DEFI-2780-11-deposit-sol-yields-to-scan branch from 57bcefc to 6a46dec Compare October 14, 2026 09:18
@gregorydemay
gregorydemay force-pushed the DEFI-2780-11-deposit-sol-yields-to-scan branch from 6a46dec to cae6c83 Compare October 14, 2026 09:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

A formatting error will fail the repository’s enforced rustfmt check.

1 open finding

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 14, 2026 09:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The concurrency handling is consistent with deposit state invariants and is adequately tested.

1 open finding

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 14, 2026 09:35
@gregorydemay
gregorydemay force-pushed the DEFI-2780-11-deposit-sol-yields-to-scan branch from cae6c83 to 5fa681f Compare October 14, 2026 09:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

A formatting error will fail the repository’s mandatory cargo fmt check.

1 open finding

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 14, 2026 09:56
@gregorydemay
gregorydemay force-pushed the DEFI-2780-11-deposit-sol-yields-to-scan branch from 5fa681f to 4907c46 Compare October 14, 2026 09:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The concurrency handling, cycle charging, and regression coverage are consistent and complete.

1 open finding

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 14, 2026 11:19
@gregorydemay
gregorydemay force-pushed the DEFI-2780-11-deposit-sol-yields-to-scan branch from 4907c46 to 50baeb3 Compare October 14, 2026 11:19
Copilot AI balanced review requested due to automatic review settings October 14, 2026 14:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

A formatting error will fail the repository’s lint check.

1 open finding

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 14, 2026 14:58
@gregorydemay
gregorydemay force-pushed the DEFI-2780-11-deposit-sol-yields-to-scan branch from b34b3f8 to 421f424 Compare October 14, 2026 14:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The concurrency handling is sound and tested; only a non-blocking formatting nit remains.

1 open finding

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 15, 2026 09:04
@gregorydemay
gregorydemay force-pushed the DEFI-2780-11-deposit-sol-yields-to-scan branch from 421f424 to 578ee32 Compare October 15, 2026 09:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused race-handling change has targeted coverage and no identified blocking issues.

1 open finding

🧠 Review effort: Balanced

@gregorydemay
gregorydemay force-pushed the DEFI-2780-11-deposit-sol-yields-to-scan branch from 578ee32 to 823cc11 Compare October 15, 2026 09:54
Copilot AI balanced review requested due to automatic review settings October 15, 2026 09:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused race check preserves read-only charging, covers both read outcomes, and has no identified blocking issues.

1 open finding

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 15, 2026 11:28
@gregorydemay
gregorydemay force-pushed the DEFI-2780-11-deposit-sol-yields-to-scan branch from 823cc11 to c84f09c Compare October 15, 2026 11:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused concurrency safeguard and charging behavior are covered by tests, with no unresolved findings.

1 open finding

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 15, 2026 12:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused fix matches the documented concurrency contract, with no blocking issues identified in static review; tests were not executed.

1 open finding

🧠 Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused guard prevents duplicate queueing, preserves read-only charging, and has targeted regression coverage.

1 open finding

🧠 Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused concurrency change has coverage for successful and rejected reads, with no blocking issues identified.

1 open finding

🧠 Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused concurrency fix has regression coverage for both read outcomes, with no unresolved findings.

1 open finding

🧠 Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request. Check if the Files changed in this pull request are included in default exclusions.

Still open (1)

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