Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Acrossv3 hotfix [AcrossFacetPackedV3 v1.2.0,ReceiverAcrossV3 v1.0.1] #897

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Acrossv3 hotfix [AcrossFacetPackedV3 v1.2.0,ReceiverAcrossV3 v1.0.1] #897

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
4b24a76
Add depositor to packed params
ezynda3 Dec 6, 2024
56aebd5
Drop gas introspection in receiver contract
ezynda3 Dec 6, 2024
d2415f5
Merge branch 'main' of github.com:lifinance/contracts into acrossv3-h…
ezynda3 Dec 6, 2024
efeab77
Deploy to staging
ezynda3 Dec 6, 2024
cf35ee4
Fix types CI job
ezynda3 Dec 6, 2024
d1f9dd9
Bump versions
ezynda3 Dec 6, 2024
5f59b49
Add comments removed by mistake
ezynda3 Dec 6, 2024
9a8ff40
Update min methods to use depositor as well
ezynda3 Dec 6, 2024
3d7eb03
Attach audit report
ezynda3 Dec 6, 2024
cd471a9
Fix log
ezynda3 Dec 6, 2024
ac60f3f
Update logs
ezynda3 Dec 9, 2024
48d3139
Deploy to zksync
ezynda3 Dec 10, 2024
d75d0a0
Update logs
ezynda3 Dec 12, 2024
20affd6
Update logs for blast and mode
ezynda3 Dec 12, 2024
34dcdc1
Update logs for linea
ezynda3 Dec 12, 2024
3bdbf13
Merge branch 'main' into acrossv3-hotfix
ezynda3 Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
173 changes: 102 additions & 71 deletions src/Facets/AcrossFacetPackedV3.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ contract AcrossFacetPackedV3 is ILiFi, TransferrableOwnership {
struct PackedParameters {
bytes32 transactionId;
address receiver;
address depositor;
ezynda3 marked this conversation as resolved.
Show resolved Hide resolved
uint64 destinationChainId;
address receivingAssetId;
uint256 outputAmount;
Expand Down Expand Up @@ -81,22 +82,34 @@ contract AcrossFacetPackedV3 is ILiFi, TransferrableOwnership {
}

/// @notice Bridges native tokens via Across (packed implementation)
/// No params, all data will be extracted from manually encoded callData
/// @dev Calldata mapping:
/// [0:4] - function selector
/// [4:12] - transactionId
/// [12:32] - receiver
/// [32:52] - depositor
/// [52:56] - destinationChainId
/// [56:76] - receivingAssetId
/// [76:108] - outputAmount
/// [108:128] - exclusiveRelayer
/// [128:132] - quoteTimestamp
/// [132:136] - fillDeadline
/// [136:140] - exclusivityDeadline
/// [140:] - message
ezynda3 marked this conversation as resolved.
Show resolved Hide resolved
function startBridgeTokensViaAcrossV3NativePacked() external payable {
// call Across spoke pool to bridge assets
spokePool.depositV3{ value: msg.value }(
msg.sender, // depositor
address(bytes20(msg.data[32:52])), // depositor
address(bytes20(msg.data[12:32])), // recipient
wrappedNative, // inputToken
address(bytes20(msg.data[36:56])), // outputToken
address(bytes20(msg.data[56:76])), // outputToken
msg.value, // inputAmount
uint256(bytes32(msg.data[56:88])), // outputAmount
uint64(uint32(bytes4(msg.data[32:36]))), // destinationChainId
address(bytes20(msg.data[88:108])), // exclusiveRelayer
uint32(bytes4(msg.data[108:112])), // quoteTimestamp
uint32(bytes4(msg.data[112:116])), // fillDeadline
uint32(bytes4(msg.data[116:120])), // exclusivityDeadline
msg.data[120:msg.data.length]
uint256(bytes32(msg.data[76:108])), // outputAmount
uint64(uint32(bytes4(msg.data[52:56]))), // destinationChainId
address(bytes20(msg.data[108:128])), // exclusiveRelayer
uint32(bytes4(msg.data[128:132])), // quoteTimestamp
uint32(bytes4(msg.data[132:136])), // fillDeadline
uint32(bytes4(msg.data[136:140])), // exclusivityDeadline
msg.data[140:msg.data.length]
);

emit LiFiAcrossTransfer(bytes8(msg.data[4:12]));
Expand Down Expand Up @@ -127,32 +140,44 @@ contract AcrossFacetPackedV3 is ILiFi, TransferrableOwnership {
}

/// @notice Bridges ERC20 tokens via Across (packed implementation)
/// No params, all data will be extracted from manually encoded callData
/// @dev Calldata mapping:
/// [0:4] - function selector
/// [4:12] - transactionId
/// [12:32] - receiver
/// [32:52] - depositor
/// [52:72] - sendingAssetId
/// [72:88] - inputAmount
/// [88:92] - destinationChainId
/// [92:112] - receivingAssetId
/// [112:144] - outputAmount
/// [144:164] - exclusiveRelayer
/// [164:168] - quoteTimestamp
/// [168:172] - fillDeadline
/// [172:176] - exclusivityDeadline
/// [176:] - message
function startBridgeTokensViaAcrossV3ERC20Packed() external {
address sendingAssetId = address(bytes20(msg.data[32:52]));
uint256 inputAmount = uint256(uint128(bytes16(msg.data[52:68])));
address sendingAssetId = address(bytes20(msg.data[52:72]));
uint256 inputAmount = uint256(uint128(bytes16(msg.data[72:88])));

// Deposit assets
ERC20(sendingAssetId).safeTransferFrom(
msg.sender,
address(this),
inputAmount
);

// call Across SpokePool
spokePool.depositV3(
msg.sender, // depositor
address(bytes20(msg.data[32:52])), // depositor
address(bytes20(msg.data[12:32])), // recipient
sendingAssetId, // inputToken
address(bytes20(msg.data[72:92])), // outputToken
address(bytes20(msg.data[92:112])), // outputToken
inputAmount, // inputAmount
uint256(bytes32(msg.data[92:124])), // outputAmount
uint64(uint32(bytes4(msg.data[68:72]))), // destinationChainId
address(bytes20(msg.data[124:144])), // exclusiveRelayer
uint32(bytes4(msg.data[144:148])), // quoteTimestamp
uint32(bytes4(msg.data[148:152])), // fillDeadline
uint32(bytes4(msg.data[152:156])), // exclusivityDeadline
msg.data[156:msg.data.length]
uint256(bytes32(msg.data[112:144])), // outputAmount
uint64(uint32(bytes4(msg.data[88:92]))), // destinationChainId
address(bytes20(msg.data[144:164])), // exclusiveRelayer
uint32(bytes4(msg.data[164:168])), // quoteTimestamp
uint32(bytes4(msg.data[168:172])), // fillDeadline
uint32(bytes4(msg.data[172:176])), // exclusivityDeadline
msg.data[176:msg.data.length]
);

emit LiFiAcrossTransfer(bytes8(msg.data[4:12]));
Expand Down Expand Up @@ -198,8 +223,6 @@ contract AcrossFacetPackedV3 is ILiFi, TransferrableOwnership {
function encode_startBridgeTokensViaAcrossV3NativePacked(
PackedParameters calldata _parameters
) external pure returns (bytes memory) {
// there are already existing networks with chainIds outside uint32 range but since we not support either of them yet,
// we feel comfortable using this approach to save further gas
require(
_parameters.destinationChainId <= type(uint32).max,
"destinationChainId value passed too big to fit in uint32"
Expand All @@ -212,6 +235,7 @@ contract AcrossFacetPackedV3 is ILiFi, TransferrableOwnership {
.selector,
bytes8(_parameters.transactionId),
bytes20(_parameters.receiver),
bytes20(_parameters.depositor),
bytes4(uint32(_parameters.destinationChainId)),
bytes20(_parameters.receivingAssetId),
bytes32(_parameters.outputAmount),
Expand All @@ -232,8 +256,6 @@ contract AcrossFacetPackedV3 is ILiFi, TransferrableOwnership {
address sendingAssetId,
uint256 inputAmount
) external pure returns (bytes memory) {
// there are already existing networks with chainIds outside uint32 range but since we not support either of them yet,
// we feel comfortable using this approach to save further gas
require(
_parameters.destinationChainId <= type(uint32).max,
"destinationChainId value passed too big to fit in uint32"
Expand All @@ -244,24 +266,33 @@ contract AcrossFacetPackedV3 is ILiFi, TransferrableOwnership {
"inputAmount value passed too big to fit in uint128"
);

return
bytes.concat(
AcrossFacetPackedV3
.startBridgeTokensViaAcrossV3ERC20Packed
.selector,
bytes8(_parameters.transactionId),
bytes20(_parameters.receiver),
bytes20(sendingAssetId),
bytes16(uint128(inputAmount)),
bytes4(uint32(_parameters.destinationChainId)),
bytes20(_parameters.receivingAssetId),
bytes32(_parameters.outputAmount),
bytes20(_parameters.exclusiveRelayer),
bytes4(_parameters.quoteTimestamp),
bytes4(_parameters.fillDeadline),
bytes4(_parameters.exclusivityDeadline),
_parameters.message
);
// Split the concatenation into parts to avoid "stack too deep" errors
bytes memory part1 = bytes.concat(
AcrossFacetPackedV3
.startBridgeTokensViaAcrossV3ERC20Packed
.selector,
bytes8(_parameters.transactionId),
bytes20(_parameters.receiver),
bytes20(_parameters.depositor),
bytes20(sendingAssetId)
);

bytes memory part2 = bytes.concat(
bytes16(uint128(inputAmount)),
bytes4(uint32(_parameters.destinationChainId)),
bytes20(_parameters.receivingAssetId),
bytes32(_parameters.outputAmount)
);

bytes memory part3 = bytes.concat(
bytes20(_parameters.exclusiveRelayer),
bytes4(_parameters.quoteTimestamp),
bytes4(_parameters.fillDeadline),
bytes4(_parameters.exclusivityDeadline)
);

// Combine all parts with the message
return bytes.concat(part1, part2, part3, _parameters.message);
}

/// @notice Decodes calldata that is meant to be used for calling the native 'packed' function
Expand All @@ -277,23 +308,24 @@ contract AcrossFacetPackedV3 is ILiFi, TransferrableOwnership {
)
{
require(
data.length >= 120,
"invalid calldata (must have length >= 120)"
data.length >= 140,
"invalid calldata (must have length >= 140)"
);

// extract bridgeData
bridgeData.transactionId = bytes32(bytes8(data[4:12]));
bridgeData.receiver = address(bytes20(data[12:32]));
bridgeData.destinationChainId = uint64(uint32(bytes4(data[32:36])));
bridgeData.destinationChainId = uint64(uint32(bytes4(data[52:56])));

// extract acrossData
acrossData.receivingAssetId = address(bytes20(data[36:56]));
acrossData.outputAmount = uint256(bytes32(data[56:88]));
acrossData.exclusiveRelayer = address(bytes20(data[88:108]));
acrossData.quoteTimestamp = uint32(bytes4(data[108:112]));
acrossData.fillDeadline = uint32(bytes4(data[112:116]));
acrossData.exclusivityDeadline = uint32(bytes4(data[116:120]));
acrossData.message = data[120:];
acrossData.refundAddress = address(bytes20(data[32:52])); // depositor
acrossData.receivingAssetId = address(bytes20(data[56:76]));
acrossData.outputAmount = uint256(bytes32(data[76:108]));
acrossData.exclusiveRelayer = address(bytes20(data[108:128]));
acrossData.quoteTimestamp = uint32(bytes4(data[128:132]));
acrossData.fillDeadline = uint32(bytes4(data[132:136]));
acrossData.exclusivityDeadline = uint32(bytes4(data[136:140]));
acrossData.message = data[140:];

return (bridgeData, acrossData);
}
Expand All @@ -311,25 +343,24 @@ contract AcrossFacetPackedV3 is ILiFi, TransferrableOwnership {
)
{
require(
data.length >= 156,
"invalid calldata (must have length > 156)"
data.length >= 176,
"invalid calldata (must have length >= 176)"
);

// extract bridgeData
bridgeData.transactionId = bytes32(bytes8(data[4:12]));
bridgeData.receiver = address(bytes20(data[12:32]));
bridgeData.sendingAssetId = address(bytes20(data[32:52]));
bridgeData.minAmount = uint256(uint128(bytes16(data[52:68])));
bridgeData.destinationChainId = uint64(uint32(bytes4(data[68:72])));

// extract acrossData
acrossData.receivingAssetId = address(bytes20(data[72:92]));
acrossData.outputAmount = uint256(bytes32(data[92:124]));
acrossData.exclusiveRelayer = address(bytes20(data[124:144]));
acrossData.quoteTimestamp = uint32(bytes4(data[144:148]));
acrossData.fillDeadline = uint32(bytes4(data[148:152]));
acrossData.exclusivityDeadline = uint32(bytes4(data[152:156]));
acrossData.message = data[156:];
acrossData.refundAddress = address(bytes20(data[32:52])); // depositor
bridgeData.sendingAssetId = address(bytes20(data[52:72]));
bridgeData.minAmount = uint256(uint128(bytes16(data[72:88])));
bridgeData.destinationChainId = uint64(uint32(bytes4(data[88:92])));

acrossData.receivingAssetId = address(bytes20(data[92:112]));
acrossData.outputAmount = uint256(bytes32(data[112:144]));
acrossData.exclusiveRelayer = address(bytes20(data[144:164]));
acrossData.quoteTimestamp = uint32(bytes4(data[164:168]));
acrossData.fillDeadline = uint32(bytes4(data[168:172]));
acrossData.exclusivityDeadline = uint32(bytes4(data[172:176]));
acrossData.message = data[176:];

return (bridgeData, acrossData);
}
Expand Down
39 changes: 8 additions & 31 deletions src/Periphery/ReceiverAcrossV3.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,9 @@ import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol";
contract ReceiverAcrossV3 is ILiFi, TransferrableOwnership {
using SafeTransferLib for address;

/// Error ///
error InsufficientGasLimit();

/// Storage ///
IExecutor public immutable executor;
address public immutable spokepool;
uint256 public immutable recoverGas;

/// Modifiers ///
modifier onlySpokepool() {
Expand All @@ -36,13 +32,11 @@ contract ReceiverAcrossV3 is ILiFi, TransferrableOwnership {
constructor(
address _owner,
address _executor,
address _spokepool,
uint256 _recoverGas
address _spokepool
) TransferrableOwnership(_owner) {
owner = _owner;
executor = IExecutor(_executor);
spokepool = _spokepool;
recoverGas = _recoverGas;
}

/// External Methods ///
Expand Down Expand Up @@ -98,6 +92,7 @@ contract ReceiverAcrossV3 is ILiFi, TransferrableOwnership {
/// Private Methods ///

/// @notice Performs a swap before completing a cross-chain transaction
// @notice Since Across will always send wrappedNative to contract, we do not need a native handling here
/// @param _transactionId the transaction id associated with the operation
/// @param _swapData array of data needed for swaps
/// @param assetId address of the token received from the source chain (not to be confused with StargateV2's assetIds which are uint16 values)
Expand All @@ -110,34 +105,16 @@ contract ReceiverAcrossV3 is ILiFi, TransferrableOwnership {
address payable receiver,
uint256 amount
) private {
// since Across will always send wrappedNative to contract, we do not need a native handling here
uint256 cacheGasLeft = gasleft();

// We introduced this handling to prevent relayers from under-estimating our destination transactions and then
// running into out-of-gas errors which would cause the bridged tokens to be refunded to the receiver. This is
// an emergency behaviour but testing showed that this would happen very frequently.
// Reverting transactions that dont have enough gas helps to make sure that transactions will get correctly estimated
// by the relayers on source chain and thus improves the success rate of destination calls.
if (cacheGasLeft < recoverGas) {
// case A: not enough gas left to execute calls
// @dev: we removed the handling to send bridged funds to receiver in case of insufficient gas
// as it's better for AcrossV3 to revert these cases instead
revert InsufficientGasLimit();
}

// case 2b: enough gas left to execute calls
assetId.safeApprove(address(executor), 0);
assetId.safeApprove(address(executor), amount);
try
executor.swapAndCompleteBridgeTokens{
gas: cacheGasLeft - recoverGas
}(_transactionId, _swapData, assetId, receiver)
executor.swapAndCompleteBridgeTokens(
_transactionId,
_swapData,
assetId,
receiver
)
{} catch {
cacheGasLeft = gasleft();
// if the only gas left here is the recoverGas then the swap must have failed due to out-of-gas error and in this
// case we want to revert (again, to force relayers to estimate our destination calls with sufficient gas limit)
if (cacheGasLeft <= recoverGas) revert InsufficientGasLimit();

// send the bridged (and unswapped) funds to receiver address
assetId.safeTransfer(receiver, amount);

Expand Down
6 changes: 4 additions & 2 deletions test/solidity/Facets/AcrossFacetPackedV3.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ contract AcrossFacetPackedV3Test is TestBase {
uint32 quoteTimestamp = uint32(block.timestamp);
validAcrossData = AcrossFacetV3.AcrossV3Data({
receiverAddress: USER_RECEIVER,
refundAddress: USER_REFUND,
refundAddress: USER_SENDER, // Set to match the depositor
receivingAssetId: ADDRESS_USDC_POL,
outputAmount: (defaultUSDCAmount * 9) / 10,
exclusiveRelayer: address(0),
Expand All @@ -147,7 +147,8 @@ contract AcrossFacetPackedV3Test is TestBase {
quoteTimestamp: quoteTimestamp,
fillDeadline: uint32(quoteTimestamp + 1000),
exclusivityDeadline: 0,
message: ""
message: "",
depositor: USER_SENDER // Add depositor field
});

vm.label(ACROSS_SPOKE_POOL, "SpokePool_PROX");
Expand Down Expand Up @@ -439,6 +440,7 @@ contract AcrossFacetPackedV3Test is TestBase {
assertEq(original.outputAmount == decoded.outputAmount, true);
assertEq(original.fillDeadline == decoded.fillDeadline, true);
assertEq(original.quoteTimestamp == decoded.quoteTimestamp, true);
assertEq(original.refundAddress == decoded.refundAddress, true); // Add check for refundAddress/depositor
assertEq(
keccak256(abi.encode(original.message)) ==
keccak256(abi.encode(decoded.message)),
Expand Down
Loading
Loading