Skip to content
This repository was archived by the owner on Sep 10, 2026. It is now read-only.

feat: add sky liquidity venue - #104

Open
Jean-Grimal wants to merge 1 commit into
mainfrom
feat/sky-liquidity-venue
Open

Jean-Grimal wants to merge 1 commit into
mainfrom
feat/sky-liquidity-venue

Conversation

@Jean-Grimal

@Jean-Grimal Jean-Grimal commented May 11, 2026

Copy link
Copy Markdown
Collaborator

@linear

linear Bot commented May 11, 2026

Copy link
Copy Markdown

SDK-109

@Foulks-Plb

Copy link
Copy Markdown
Collaborator

@codex review

@Rubilmax

Copy link
Copy Markdown
Collaborator

@prd-carapulse[bot] review this PR, focusing on security of the liquidation bot and controller wallet as well as on edge cases related to sky liquidity and markets

@prd-carapulse

prd-carapulse Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

API call failed after 3 retries: You exceeded your current quota, please check your plan and billing details. For more information on this error, read the docs: https://platform.openai.com/docs/guides/error-codes/api-errors.

Track: https://carapulse.morpho.dev/dashboard/runs?run_id=gw_run_5543923974e6

@Rubilmax

Copy link
Copy Markdown
Collaborator

@prd-carapulse[bot] review this PR, focusing on security of the liquidation bot and controller wallet as well as on edge cases related to sky liquidity and markets

@prd-carapulse

prd-carapulse Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Review posted: #104 (comment)

Result:

  • No blocking security findings.
  • Controller wallet not newly exposed.
  • Sky converter routes look correct.
  • pnpm build passed.
  • Focused fork test inconclusive due local Anvil/Foundry RPC-chain issue; direct mainnet RPC checks passed.
  • No code PR opened; only optional hardening noted.

Track: https://carapulse.morpho.dev/dashboard/runs?run_id=gw_run_700b694c9cd9

@prd-carapulse

prd-carapulse Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Review complete — no blocking security findings.

  • Controller wallet exposure: no new arbitrary calldata path. Sky converter calls are fixed selectors to configured mainnet converter addresses, value = 0, and converted assets return to encoder.address, not the controller EOA.
  • Approvals: DAI/USDS exact approval is consumed by transferFrom. MKR/SKY approval is redundant because MkrSky burns msg.sender directly; it may leave a stale exact allowance to the immutable converter, but I do not see an exploitable spend path.
  • Sky route edge cases: USDS/SKY preferAlternative chaining looks correct (USDS/SKY -> DAI/MKR -> downstream venue), direct pairs stop when src === dst, and mainnet-only config is respected.
  • On-chain sanity: mainnet RPC shows both converters have code; MkrSky.rate() returns 24000.

Verification:

  • pnpm build passed.
  • Focused fork test was inconclusive in this runtime: local Anvil/Foundry forked Base despite a mainnet fork URL, so MkrSky.rate() returned no data there. Direct mainnet RPC checks above passed.

Optional hardening: remove the redundant MKR/SKY approval and add tests for MKR -> SKY, non-multiple SKY dust rounding, and full preferAlternative chaining into a downstream venue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants