[WIP] Cross-chain Strategy #2715

shahthepro · 2025-12-11T10:38:06Z

Code Changes

Check readme: https://github.com/OriginProtocol/origin-dollar/blob/shah/cross-chain-strategy-cctpv2/contracts/contracts/strategies/crosschain/crosschain-strategy.md

codecov · 2025-12-12T11:28:20Z

Codecov Report

❌ Patch coverage is 84.45748% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.75%. Comparing base (172f9c4) to head (b16c7fd).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
...strategies/crosschain/CrossChainRemoteStrategy.sol	73.62%	24 Missing ⚠️
...strategies/crosschain/CrossChainMasterStrategy.sol	81.60%	16 Missing ⚠️
...s/strategies/crosschain/AbstractCCTPIntegrator.sol	90.65%	9 Missing and 1 partial ⚠️
...s/contracts/strategies/Generalized4626Strategy.sol	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2715      +/-   ##
==========================================
+ Coverage   41.10%   41.75%   +0.64%     
==========================================
  Files         126      133       +7     
  Lines        5778     6129     +351     
  Branches     1537     1633      +96     
==========================================
+ Hits         2375     2559     +184     
- Misses       3401     3567     +166     
- Partials        2        3       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sparrowDom · 2025-12-15T11:52:03Z

contracts/contracts/mocks/crosschain/CrossChainRemoteStrategyMock.sol

+pragma solidity ^0.8.0;
+
+/**
+ * @title OUSD Yearn V3 Remote Strategy Mock - the Mainnet part


"- the Mainnet part" should probably be "- the L2 chain part"

contracts/contracts/mocks/crosschain/CrossChainRemoteStrategyMock.sol

sparrowDom · 2025-12-16T12:05:01Z

contracts/contracts/strategies/crosschain/AbstractCCTPIntegrator.sol

+    address public immutable cctpHookWrapper;
+
+    // USDC address on local chain
+    address public immutable baseToken;


we can rename this to usdcToken?

That's a good point. Since we would only be dealing with USDC with CCTP

contracts/contracts/strategies/crosschain/AbstractCCTPIntegrator.sol

* fix deploy files * minor rename * add calls to Morpho Vault into a try catch * refactor hook wrapper * don't revert if withdraw from underlying fails * use checkBalance for deposit/withdrawal acknowledgment * update message in remote strategy * remove unneeded functions

* Fix race condition * Transfer everything on wtihdrawal * Move destination domain one step above

* add cross chain unit test basic files * add basic unit test setup * add header encoding * more tests

* more unit test integration * more tying up ends * fix bug * cleanup * add full round-trip test * cleanup * Fix approve all and prettify --------- Co-authored-by: Shahul Hameed <10547529+shahthepro@users.noreply.github.com>

naddison36

This is my first pass of the contracts

naddison36 · 2026-01-02T04:29:01Z

contracts/contracts/strategies/crosschain/CrossChainMasterStrategy.sol

+    using SafeERC20 for IERC20;
+    using CrossChainStrategyHelper for bytes;
+
+    // Remote strategy balance


nit: lets make this Natspec with /// @notice

naddison36 · 2026-01-02T04:29:19Z

contracts/contracts/strategies/crosschain/CrossChainMasterStrategy.sol

+    // Remote strategy balance
+    uint256 public remoteStrategyBalance;
+
+    // Amount that's bridged but not yet received on the destination chain


nit: can be Natspec with /// @notice

naddison36 · 2026-01-02T05:21:17Z

contracts/contracts/strategies/crosschain/AbstractCCTPIntegrator.sol

+    // USDC address on local chain
+    address public immutable baseToken;
+
+    // Domain ID of the chain from which messages are accepted


nit: best to make all these comments on public variables a Natspec with /// @notice

naddison36 · 2026-01-02T05:22:42Z

contracts/contracts/strategies/crosschain/AbstractCCTPIntegrator.sol

+    // Domain ID of the chain from which messages are accepted
+    uint32 public immutable peerDomainID;
+
+    // Strategy address on other chain


nit: I'd add the example values. eg Ethereum 0, Base 6

naddison36 · 2026-01-02T05:24:54Z

contracts/contracts/strategies/crosschain/AbstractCCTPIntegrator.sol

+    // Strategy address on other chain
+    address public immutable peerStrategy;
+
+    // CCTP params


nit: a link to the CCTP
Finality thresholds would be good here https://developers.circle.com/cctp/technical-guide#finality-thresholds

naddison36 · 2026-01-02T06:07:00Z

contracts/contracts/strategies/crosschain/CrossChainMasterStrategy.sol

+        }
+    }
+
+    /// @inheritdoc InitializableAbstractStrategy


nit: I think its better to have CrossChainMasterStrategy specific Natspec. eg _recipient must be the vault and _asset must be USDC.

naddison36 · 2026-01-02T06:10:57Z

contracts/contracts/strategies/crosschain/CrossChainMasterStrategy.sol

+     */
+    function _deposit(address _asset, uint256 depositAmount) internal virtual {
+        require(_asset == baseToken, "Unsupported asset");
+        require(!isTransferPending(), "Transfer already pending");


nit: it feels safer to have this check in _getNextNonce

naddison36 · 2026-01-02T06:13:02Z

contracts/contracts/strategies/crosschain/CrossChainMasterStrategy.sol

+        require(_asset == baseToken, "Unsupported asset");
+        require(!isTransferPending(), "Transfer already pending");
+        require(pendingAmount == 0, "Unexpected pending amount");
+        require(depositAmount > 0, "Deposit amount must be greater than 0");


nit: can be shortened to Must deposit something which is what a lot of other strategies use

naddison36 · 2026-01-02T06:14:20Z

contracts/contracts/strategies/crosschain/CrossChainMasterStrategy.sol

+        require(depositAmount > 0, "Deposit amount must be greater than 0");
+        require(
+            depositAmount <= MAX_TRANSFER_AMOUNT,
+            "Deposit amount exceeds max transfer amount"


nit: can be shortened to Deposit amount exceeds max so it fits in an EVM word

naddison36 · 2026-01-02T06:28:55Z

contracts/contracts/strategies/crosschain/CrossChainRemoteStrategy.sol

+    }
+
+    /// @inheritdoc Generalized4626Strategy
+    function deposit(address _asset, uint256 _amount)


Are the deposit, depositAll, withdraw and withdrawAll functions required on CrossChainRemoteStrategy? Won't all deposits and withdrawals come from the Ethereum via CCTP?

Copy-pasting my reply from Discord:

Yeah, that was one of the design decisions. There could be cases where the Guardian might wanna do withdrawals and deposits. Like let's say there're problems with underlying 4626 Vault and we wanna remove all our funds from that Vault instantly, the guradian can call withdraw all directly. Also, if the deposit or withdrawal fails, the guardian can do it manually without it having to be a CCTP message

naddison36 · 2026-01-02T06:50:53Z

contracts/contracts/strategies/crosschain/CrossChainRemoteStrategy.sol

+
+    /**
+     * @notice Set address of Strategist.
+     *         This is important to have the function name same as IVault.


Is this saying the Strategist needs to be the vaultAddress?

The constructor states

// Vault address must always be the proxy address // so that IVault(vaultAddress).strategistAddr() works

Nope. I think I could've worded this better, I'll change it.

So, the problem is that the Generalized4626Strategy uses IVault.strategistAddr(). However the remote strategy has no Vault, it only communicates with the Master strategy. So, whenever IVault.strategistAddr() is used in the abstract strategy, it'll revert. So, what I did is add a function with same signature as strategistAddr(), so whenever IVault.strategistAddr() is called, it'll be same as calling address(this).strategistAddr().

Strategist should always be the 2/8 multisig. But for Remote strategy, the Vault should be its own proxy addresss

clement-ux

First surface pass

clement-ux · 2025-12-29T07:24:20Z

contracts/contracts/strategies/crosschain/AbstractCCTPIntegrator.sol

+    // Operator address: Can relay CCTP messages
+    address public operator;


Global comment about slot arrangement, maybe we can:

Move MAX_TRANSFER_AMOUNT above cctpMessageTransmitter to keep order: constant -> immutable -> variable

cast minFinalityThreshold and feePremiumBps into uint16.

move nonceProcessed bellow operator

pack minFinalityThreshold, feePremiumBps and lastTransferNonce all together in a single slot.

clement-ux · 2025-12-29T07:39:16Z

contracts/contracts/strategies/crosschain/AbstractCCTPIntegrator.sol

+        require(
+            minFinalityThreshold == 1000,
+            "Unfinalized messages are not supported"
+        );


Quick question, what's the reason for only supporting fast transfer and not standard transfer?

clement-ux · 2026-01-02T09:38:04Z

contracts/contracts/strategies/crosschain/AbstractCCTPIntegrator.sol

+// CCTP Message Header fields
+// Ref: https://developers.circle.com/cctp/technical-guide#message-header
+uint8 constant VERSION_INDEX = 0;
+uint8 constant SOURCE_DOMAIN_INDEX = 4;
+uint8 constant SENDER_INDEX = 44;
+uint8 constant RECIPIENT_INDEX = 76;
+uint8 constant MESSAGE_BODY_INDEX = 148;
+
+// Message body V2 fields
+// Ref: https://developers.circle.com/cctp/technical-guide#message-body
+// Ref: https://github.com/circlefin/evm-cctp-contracts/blob/master/src/messages/v2/BurnMessageV2.sol
+uint8 constant BURN_MESSAGE_V2_VERSION_INDEX = 0;
+uint8 constant BURN_MESSAGE_V2_RECIPIENT_INDEX = 36;
+uint8 constant BURN_MESSAGE_V2_AMOUNT_INDEX = 68;
+uint8 constant BURN_MESSAGE_V2_MESSAGE_SENDER_INDEX = 100;
+uint8 constant BURN_MESSAGE_V2_FEE_EXECUTED_INDEX = 164;
+uint8 constant BURN_MESSAGE_V2_HOOK_DATA_INDEX = 228;


nit: Shouldn't we create a Library for these constants?

clement-ux · 2026-01-02T09:41:55Z

contracts/contracts/strategies/crosschain/CrossChainMasterStrategy.sol

+    enum TransferType {
+        None, // To avoid using 0
+        Deposit,
+        Withdrawal
+    }


I think we should have None, Deposit, Withdrawal in capital letter.

clement-ux · 2026-01-02T09:48:16Z

contracts/contracts/strategies/crosschain/CrossChainMasterStrategy.sol

+        uint256 undepositedUSDC = IERC20(baseToken).balanceOf(address(this));
+        return undepositedUSDC + pendingAmount + remoteStrategyBalance;


nit: I don't think it is necessary to cache undepositedUSDC, we can directly have:

return IERC20(baseToken).balanceOf(address(this)) + pendingAmount + remoteStrategyBalance;

clement-ux · 2026-01-02T10:23:08Z

contracts/contracts/strategies/crosschain/CrossChainMasterStrategy.sol

+            _markNonceAsProcessed(nonce);
+
+            // Effect of confirming a deposit, reset pending amount
+            pendingAmount = 0;


gas: delete pendingAmount should be cheaper than pendingAmount = 0;

clement-ux · 2026-01-02T10:29:35Z

contracts/contracts/strategies/crosschain/CrossChainRemoteStrategy.sol

+    /// @inheritdoc Generalized4626Strategy
+    function withdrawAll() external virtual override onlyGovernorOrStrategist {
+        uint256 contractBalance = IERC20(baseToken).balanceOf(address(this));
+        uint256 balance = checkBalance(baseToken) - contractBalance;
+        _withdraw(address(this), baseToken, balance);
+    }


It fetches IERC20(baseToken).balanceOf(address(this)) two times, one at line 139, once again in checkBalance.
Maybe we can have something like this:

function withdrawAll() external virtual override onlyGovernorOrStrategist { _withdraw(address(this), baseToken, IERC4626(platformAddress).previewRedeem(platform.balanceOf(address(this)))); }

sparrowDom and others added 5 commits December 5, 2025 14:54

some scaffolding

637fb40

add basic necessities for unit tests

7630f6d

checkpoint

6a97767

Fix compiling issues

9517fca

Add fork test scaffolding

41f1fd9

shahthepro added 5 commits December 13, 2025 00:30

Fix stuffs

7a109cc

Prettify and change salt

48d317e

Add auto-verification

c6a254a

Fix checkBalance

8f4e39e

Make CCTPHookWrapper more resilient

f7a9b97

sparrowDom reviewed Dec 15, 2025

View reviewed changes

contracts/contracts/mocks/crosschain/CrossChainRemoteStrategyMock.sol Outdated Show resolved Hide resolved

refactor message version and type checks

b3c1eb4

sparrowDom reviewed Dec 16, 2025

View reviewed changes

contracts/contracts/strategies/crosschain/AbstractCCTPIntegrator.sol Show resolved Hide resolved

sparrowDom reviewed Dec 16, 2025

View reviewed changes

contracts/contracts/strategies/crosschain/AbstractCCTPIntegrator.sol Show resolved Hide resolved

sparrowDom reviewed Dec 16, 2025

View reviewed changes

contracts/contracts/strategies/crosschain/AbstractCCTPIntegrator.sol Show resolved Hide resolved

sparrowDom reviewed Dec 16, 2025

View reviewed changes

contracts/contracts/strategies/crosschain/AbstractCCTPIntegrator.sol Outdated Show resolved Hide resolved

sparrowDom and others added 11 commits December 16, 2025 16:50

add some comments

d3c4a39

add comment

792c890

fix compile errors

70166bc

Change addresses

1e700b1

Merge branch 'master' into shah/cross-chain-strategy-cctpv2

ada1546

Fix compilation issues

987dc0a

Fix deployment files a bit

b63bd5f

Fix Message relayer

bf1fbe2

Clean up master strategy

a0dd07b

Fix deployment file name

4071943

shahthepro and others added 22 commits December 18, 2025 14:08

move around stuff

9e36485

Fix CCTP Integrator

e401fa1

clean up fork

73abf6c

Fix race condition (#2720)

266ec01

* Fix race condition * Transfer everything on wtihdrawal * Move destination domain one step above

Cleanup code

122fa32

decode payloads in fork tests

7507994

Add library for message handling

7998ffa

More changes

e34706a

Add comments and prettify

df59d53

WIP Unit test setup (#2722)

29ee0e6

* add cross chain unit test basic files * add basic unit test setup * add header encoding * more tests

Add more fork tests

a0d5379

Add token transfer tests

3585d9d

WIP Unit tests for OUSD Simplified strategy (#2724)

e694906

* more unit test integration * more tying up ends * fix bug * cleanup * add full round-trip test * cleanup * Fix approve all and prettify --------- Co-authored-by: Shahul Hameed <10547529+shahthepro@users.noreply.github.com>

Fix master fork tests

77bc730

linter

6dca3cc

add direct withdrawal paths and additional checks

7de74b4

Fix Remote strategy tests

fb31b79

Update comments and clean up code

54ca3c0

Fix comment

5ac664c

Fix failing unit test

b16c7fd

fix: withdraw only if balance is lower than requested amount

a4c2968

Document crosschain strategy

fca3df2

naddison36 reviewed Jan 2, 2026

View reviewed changes

shahthepro added 2 commits January 2, 2026 10:36

Merge branch 'master' into shah/cross-chain-strategy-cctpv2

e322fe3

Update deployment file numbers

b2feed1

naddison36 reviewed Jan 2, 2026

View reviewed changes

clement-ux reviewed Jan 2, 2026

View reviewed changes

		// Operator address: Can relay CCTP messages
		address public operator;

		uint256 undepositedUSDC = IERC20(baseToken).balanceOf(address(this));
		return undepositedUSDC + pendingAmount + remoteStrategyBalance;

[WIP] Cross-chain Strategy #2715

Are you sure you want to change the base?

[WIP] Cross-chain Strategy #2715

Conversation

shahthepro commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Changes

Uh oh!

codecov bot commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

naddison36 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clement-ux left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

shahthepro commented Dec 11, 2025 •

edited

Loading

codecov bot commented Dec 12, 2025 •

edited

Loading