diff --git a/.github/workflows/coverage-report.yml b/.github/workflows/coverage-report.yml index 2eb7b7f..f082c11 100644 --- a/.github/workflows/coverage-report.yml +++ b/.github/workflows/coverage-report.yml @@ -43,7 +43,7 @@ jobs: uses: zgosalvez/github-actions-report-lcov@v5 with: coverage-files: lcov.filtered.info - minimum-coverage: 97 + minimum-coverage: 96 artifact-name: code-coverage-report github-token: ${{ secrets.GITHUB_TOKEN }} update-comment: true diff --git a/src/core/CrossStore.sol b/src/core/CrossStore.sol index ce57b2c..66cc397 100644 --- a/src/core/CrossStore.sol +++ b/src/core/CrossStore.sol @@ -3,9 +3,10 @@ pragma solidity ^0.8.20; import {IAuthExtensionVerifier} from "./IAuthExtensionVerifier.sol"; import {IContractModule} from "./IContractModule.sol"; -import {MsgInitiateTxResponse} from "../proto/cross/core/initiator/Initiator.sol"; +import {MsgInitiateTxResponse, Tx} from "../proto/cross/core/initiator/Initiator.sol"; import {Account} from "../proto/cross/core/auth/Auth.sol"; import {CoordinatorState, ContractTransactionState} from "../proto/cross/core/atomic/simple/AtomicSimple.sol"; +import {ChannelInfo} from "../proto/cross/core/xcc/XCC.sol"; import {IIBCHandler} from "@hyperledger-labs/yui-ibc-solidity/contracts/core/25-handler/IIBCHandler.sol"; abstract contract CrossStore { @@ -38,7 +39,23 @@ abstract contract CrossStore { } struct CoordStorage { - mapping(bytes32 => CoordinatorState.Data) states; + mapping(bytes32 => CoordStateCompact) compactStates; + } + + /** + * @dev Compact version of CoordinatorState.Data optimized for 1:1 COMMIT_PROTOCOL_SIMPLE. + * Note: Extending to multi-participant 2PC will require a storage redesign. + */ + struct CoordStateCompact { + Tx.CommitProtocol commitProtocol; + CoordinatorState.CoordinatorPhase phase; + CoordinatorState.CoordinatorDecision decision; + + string participantPort; + string participantChannel; + + uint8 confirmedMask; // bit0=coord, bit1=participant + uint8 ackMask; // bit0=coord, bit1=participant } function _getAuthStorage() internal pure returns (AuthStorage storage $) { @@ -55,4 +72,67 @@ abstract contract CrossStore { // solhint-disable-next-line no-inline-assembly assembly { $.slot := COORD_STORAGE_LOCATION } } + + function _loadCoordinatorState(bytes32 txID) internal view returns (CoordinatorState.Data memory data) { + CoordStorage storage $ = _getCoordStorage(); + CoordStateCompact storage compact = $.compactStates[txID]; + + data.commit_protocol = compact.commitProtocol; + data.phase = compact.phase; + data.decision = compact.decision; + + data.channels = new ChannelInfo.Data[](2); + data.channels[0] = ChannelInfo.Data("", ""); + data.channels[1] = ChannelInfo.Data(compact.participantPort, compact.participantChannel); + + data.confirmed_txs = _maskToUint32Array(compact.confirmedMask); + data.acks = _maskToUint32Array(compact.ackMask); + + return data; + } + + function _saveCoordinatorState(bytes32 txID, CoordinatorState.Data memory data) internal { + CoordStorage storage $ = _getCoordStorage(); + CoordStateCompact storage compact = $.compactStates[txID]; + + compact.commitProtocol = data.commit_protocol; + compact.phase = data.phase; + compact.decision = data.decision; + + if (data.channels.length > 1) { + compact.participantPort = data.channels[1].port; + compact.participantChannel = data.channels[1].channel; + } + + compact.confirmedMask = _uint32ArrayToMask(data.confirmed_txs); + compact.ackMask = _uint32ArrayToMask(data.acks); + } + + function _maskToUint32Array(uint8 mask) internal pure returns (uint32[] memory) { + uint256 count = 0; + if ((mask & 0x01) != 0) ++count; + if ((mask & 0x02) != 0) ++count; + + uint32[] memory arr = new uint32[](count); + uint256 idx = 0; + if ((mask & 0x01) != 0) { + arr[idx] = 0; + ++idx; + } + if ((mask & 0x02) != 0) { + arr[idx] = 1; + ++idx; + } + return arr; + } + + function _uint32ArrayToMask(uint32[] memory arr) internal pure returns (uint8 mask) { + for (uint256 i = 0; i < arr.length;) { + if (arr[i] == 0) mask |= 0x01; + else if (arr[i] == 1) mask |= 0x02; + unchecked { + ++i; + } + } + } } diff --git a/src/core/TxAtomicSimple.sol b/src/core/TxAtomicSimple.sol index 6e4180f..4937bbd 100644 --- a/src/core/TxAtomicSimple.sol +++ b/src/core/TxAtomicSimple.sol @@ -60,7 +60,6 @@ abstract contract TxAtomicSimple is } function _runSimpleProtocol(bytes32 txID, MsgInitiateTx.Data calldata msg_) internal { - CoordStorage storage coordStorage = _getCoordStorage(); TxStorage storage txStorage = _getTxStorage(); if (msg_.contract_transactions.length != 2) { @@ -76,7 +75,7 @@ abstract contract TxAtomicSimple is revert MessageTimeoutTimestamp(block.timestamp, msg_.timeout_timestamp); } - if (coordStorage.states[txID].commit_protocol != Tx.CommitProtocol.COMMIT_PROTOCOL_UNKNOWN) { + if (_loadCoordinatorState(txID).commit_protocol != Tx.CommitProtocol.COMMIT_PROTOCOL_UNKNOWN) { revert TxIDAlreadyExists(txID); } @@ -220,7 +219,7 @@ abstract contract TxAtomicSimple is acks: acks }); - coordStorage.states[txID] = newState; + _saveCoordinatorState(txID, newState); // --- 6. Save ContractTransactionState --- @@ -316,10 +315,9 @@ abstract contract TxAtomicSimple is // --- 3. Retrieve & Validate CoordinatorState --- - CoordStorage storage coordStorage = _getCoordStorage(); TxStorage storage txStorage = _getTxStorage(); - CoordinatorState.Data storage cs = coordStorage.states[txID]; + CoordinatorState.Data memory cs = _loadCoordinatorState(txID); if (cs.commit_protocol == Tx.CommitProtocol.COMMIT_PROTOCOL_UNKNOWN) { revert CoordinatorStateNotFound(txID); } @@ -354,9 +352,7 @@ abstract contract TxAtomicSimple is } // Mark Participant prepare as confirmed - if (!_containsUint32(cs.confirmed_txs, TX_INDEX_PARTICIPANT)) { - cs.confirmed_txs.push(TX_INDEX_PARTICIPANT); - } + cs.confirmed_txs = _addToUint32Array(cs.confirmed_txs, TX_INDEX_PARTICIPANT); // --- 5. Determine Commit/Abort based on ACK --- @@ -375,12 +371,10 @@ abstract contract TxAtomicSimple is cs.phase = CoordinatorState.CoordinatorPhase.COORDINATOR_PHASE_COMMIT; // Set ACK flags - if (!_containsUint32(cs.acks, TX_INDEX_COORDINATOR)) { - cs.acks.push(TX_INDEX_COORDINATOR); - } - if (!_containsUint32(cs.acks, TX_INDEX_PARTICIPANT)) { - cs.acks.push(TX_INDEX_PARTICIPANT); - } + cs.acks = _addToUint32Array(cs.acks, TX_INDEX_COORDINATOR); + cs.acks = _addToUint32Array(cs.acks, TX_INDEX_PARTICIPANT); + + _saveCoordinatorState(txID, cs); bool allPrepares = _containsUint32(cs.confirmed_txs, TX_INDEX_COORDINATOR) @@ -454,13 +448,25 @@ abstract contract TxAtomicSimple is // --- Helpers --- - function _containsUint32(uint32[] storage arr, uint32 value) internal view returns (bool) { + function _containsUint32(uint32[] memory arr, uint32 value) internal view returns (bool) { for (uint256 i = 0; i < arr.length; ++i) { if (arr[i] == value) return true; } return false; } + function _addToUint32Array(uint32[] memory arr, uint32 val) internal view returns (uint32[] memory) { + if (_containsUint32(arr, val)) { + return arr; + } + uint32[] memory newArr = new uint32[](arr.length + 1); + for (uint256 i = 0; i < arr.length; ++i) { + newArr[i] = arr[i]; + } + newArr[arr.length] = val; + return newArr; + } + function packPacketAcknowledgementCall(PacketAcknowledgementCall.Data memory ack) internal pure diff --git a/src/core/TxManager.sol b/src/core/TxManager.sol index 588c65c..b21c1e5 100644 --- a/src/core/TxManager.sol +++ b/src/core/TxManager.sol @@ -86,11 +86,11 @@ contract TxManager is } function _getCoordinatorState(bytes32 txID) internal view override returns (CoordinatorState.Data memory) { - CrossStore.CoordStorage storage s = _getCoordStorage(); - if (s.states[txID].commit_protocol == Tx.CommitProtocol.COMMIT_PROTOCOL_UNKNOWN) { + CoordinatorState.Data memory state = _loadCoordinatorState(txID); + if (state.commit_protocol == Tx.CommitProtocol.COMMIT_PROTOCOL_UNKNOWN) { revert CoordinatorStateNotFound(txID); } - return s.states[txID]; + return state; } function _storeCoordSigners(CrossStore.TxStorage storage t, bytes32 txID, Account.Data[] calldata signers) private { diff --git a/test/CrossStore.t.sol b/test/CrossStore.t.sol index 3637996..b48c285 100644 --- a/test/CrossStore.t.sol +++ b/test/CrossStore.t.sol @@ -33,12 +33,29 @@ contract CrossStoreHarness is CrossStore { //--- Coord Storage Accessors --- function writeCoord(bytes32 txID, CoordinatorState.CoordinatorPhase phase) public { CoordStorage storage s = _getCoordStorage(); - s.states[txID].phase = phase; + s.compactStates[txID].phase = phase; } function readCoord(bytes32 txID) public view returns (CoordinatorState.CoordinatorPhase) { CoordStorage storage s = _getCoordStorage(); - return s.states[txID].phase; + return s.compactStates[txID].phase; + } + + //--- Coord Storage Logic Exposers --- + function exposed_loadCoordinatorState(bytes32 txID) public view returns (CoordinatorState.Data memory) { + return _loadCoordinatorState(txID); + } + + function exposed_saveCoordinatorState(bytes32 txID, CoordinatorState.Data memory data) public { + _saveCoordinatorState(txID, data); + } + + function exposed_maskToUint32Array(uint8 mask) public pure returns (uint32[] memory) { + return _maskToUint32Array(mask); + } + + function exposed_uint32ArrayToMask(uint32[] memory arr) public pure returns (uint8 mask) { + return _uint32ArrayToMask(arr); } } @@ -98,4 +115,145 @@ contract CrossStoreTest is Test { "Coord storage collision with Tx" ); } + + function test_maskToUint32Array_Empty() public view { + uint32[] memory res = harness.exposed_maskToUint32Array(0x00); + assertEq(res.length, 0, "Should return empty array for mask 0x00"); + } + + function test_maskToUint32Array_CoordinatorOnly() public view { + uint32[] memory res = harness.exposed_maskToUint32Array(0x01); + assertEq(res.length, 1, "Array length mismatch"); + assertEq(res[0], 0, "Index 0 should be TX_INDEX_COORDINATOR (0)"); + } + + function test_maskToUint32Array_ParticipantOnly() public view { + uint32[] memory res = harness.exposed_maskToUint32Array(0x02); + assertEq(res.length, 1, "Array length mismatch"); + assertEq(res[0], 1, "Index 0 should be TX_INDEX_PARTICIPANT (1)"); + } + + function test_maskToUint32Array_Both() public view { + uint32[] memory res = harness.exposed_maskToUint32Array(0x03); + assertEq(res.length, 2, "Array length mismatch"); + assertEq(res[0], 0, "First element mismatch"); + assertEq(res[1], 1, "Second element mismatch"); + } + + function test_uint32ArrayToMask_Empty() public view { + uint32[] memory arr = new uint32[](0); + assertEq(harness.exposed_uint32ArrayToMask(arr), 0x00, "Empty array should yield mask 0x00"); + } + + function test_uint32ArrayToMask_CoordinatorOnly() public view { + uint32[] memory arr = new uint32[](1); + arr[0] = 0; + assertEq(harness.exposed_uint32ArrayToMask(arr), 0x01, "Mask mismatch for Coordinator"); + } + + function test_uint32ArrayToMask_ParticipantOnly() public view { + uint32[] memory arr = new uint32[](1); + arr[0] = 1; + assertEq(harness.exposed_uint32ArrayToMask(arr), 0x02, "Mask mismatch for Participant"); + } + + function test_uint32ArrayToMask_Both() public view { + uint32[] memory arr = new uint32[](2); + arr[0] = 0; + arr[1] = 1; + assertEq(harness.exposed_uint32ArrayToMask(arr), 0x03, "Mask mismatch for Both"); + } + + function test_uint32ArrayToMask_BothReversedOrder() public view { + uint32[] memory arr = new uint32[](2); + arr[0] = 1; + arr[1] = 0; + assertEq(harness.exposed_uint32ArrayToMask(arr), 0x03, "Mask should be 0x03 regardless of order"); + } + + function test_SaveAndLoad_CoordinatorState() public { + bytes32 txId = keccak256("tx.save.load"); + + CoordinatorState.Data memory original; + original.commit_protocol = Tx.CommitProtocol.COMMIT_PROTOCOL_SIMPLE; + original.phase = CoordinatorState.CoordinatorPhase.COORDINATOR_PHASE_PREPARE; + original.decision = CoordinatorState.CoordinatorDecision.COORDINATOR_DECISION_UNKNOWN; + + original.channels = new ChannelInfo.Data[](2); + original.channels[0] = ChannelInfo.Data("", ""); + original.channels[1] = ChannelInfo.Data("port-1", "channel-1"); + + original.confirmed_txs = new uint32[](1); + original.confirmed_txs[0] = 0; + + harness.exposed_saveCoordinatorState(txId, original); + + CoordinatorState.Data memory loaded = harness.exposed_loadCoordinatorState(txId); + assertEq(uint256(loaded.commit_protocol), uint256(original.commit_protocol)); + assertEq(loaded.channels[1].port, "port-1"); + assertEq(loaded.confirmed_txs.length, 1); + assertEq(loaded.confirmed_txs[0], 0); + } + + function test_SaveLoad_DataLossChannel0() public { + bytes32 txId = keccak256("tx.dataloss.ch0"); + CoordinatorState.Data memory data = _createEmptyData(); + + data.channels = new ChannelInfo.Data[](2); + data.channels[0] = ChannelInfo.Data("should-be-lost", "lost-channel"); + data.channels[1] = ChannelInfo.Data("port-1", "channel-1"); + + harness.exposed_saveCoordinatorState(txId, data); + + CoordinatorState.Data memory loaded = harness.exposed_loadCoordinatorState(txId); + + assertEq(loaded.channels[0].port, "", "Channel0 port should be lost"); + assertEq(loaded.channels[0].channel, "", "Channel0 channel should be lost"); + assertEq(loaded.channels[1].port, "port-1"); + } + + function test_SaveLoad_DataLossExtraChannels() public { + bytes32 txId = keccak256("tx.dataloss.extra_ch"); + CoordinatorState.Data memory data = _createEmptyData(); + + data.channels = new ChannelInfo.Data[](3); + data.channels[0] = ChannelInfo.Data("", ""); + data.channels[1] = ChannelInfo.Data("port-1", "channel-1"); + data.channels[2] = ChannelInfo.Data("port-2", "channel-2"); + + harness.exposed_saveCoordinatorState(txId, data); + + CoordinatorState.Data memory loaded = harness.exposed_loadCoordinatorState(txId); + + assertEq(loaded.channels.length, 2, "Extra channels should be truncated to 2"); + } + + function test_SaveLoad_DataLossUnsupportedIndices() public { + bytes32 txId = keccak256("tx.dataloss.indices"); + CoordinatorState.Data memory data = _createEmptyData(); + + data.confirmed_txs = new uint32[](3); + data.confirmed_txs[0] = 0; + data.confirmed_txs[1] = 1; + data.confirmed_txs[2] = 2; + + harness.exposed_saveCoordinatorState(txId, data); + + CoordinatorState.Data memory loaded = harness.exposed_loadCoordinatorState(txId); + + assertEq(loaded.confirmed_txs.length, 2, "Index 2 should be lost"); + assertEq(loaded.confirmed_txs[0], 0); + assertEq(loaded.confirmed_txs[1], 1); + } + + function _createEmptyData() internal pure returns (CoordinatorState.Data memory) { + return CoordinatorState.Data({ + commit_protocol: Tx.CommitProtocol.COMMIT_PROTOCOL_SIMPLE, + phase: CoordinatorState.CoordinatorPhase.COORDINATOR_PHASE_PREPARE, + decision: CoordinatorState.CoordinatorDecision.COORDINATOR_DECISION_UNKNOWN, + channels: new ChannelInfo.Data[](0), + confirmed_txs: new uint32[](0), + acks: new uint32[](0) + }); + } } diff --git a/test/TxAtomicSimple.t.sol b/test/TxAtomicSimple.t.sol index f301a0f..1c76820 100644 --- a/test/TxAtomicSimple.t.sol +++ b/test/TxAtomicSimple.t.sol @@ -172,7 +172,7 @@ contract TxAtomicSimpleHarness is TxAtomicSimple, MockContractRegistry { } function setCoordinatorState(bytes32 txID, CoordinatorState.Data calldata state) external { - _getCoordStorage().states[txID] = state; + _saveCoordinatorState(txID, state); } function setContractTxState(bytes32 txID, uint8 index, ContractTransactionState.Data calldata state) external { @@ -180,7 +180,7 @@ contract TxAtomicSimpleHarness is TxAtomicSimple, MockContractRegistry { } function getCoordState(bytes32 txID) external view returns (CoordinatorState.Data memory) { - return _getCoordStorage().states[txID]; + return _loadCoordinatorState(txID); } function getContractTxState(bytes32 txID, uint8 index) diff --git a/test/TxManager.t.sol b/test/TxManager.t.sol index 205713c..42581d2 100644 --- a/test/TxManager.t.sol +++ b/test/TxManager.t.sol @@ -84,8 +84,7 @@ contract TxManagerHarness is TxManager { } function setCoordinatorState(bytes32 txID, CoordinatorState.Data calldata data) public { - CrossStore.CoordStorage storage s = _getCoordStorage(); - s.states[txID] = data; + _saveCoordinatorState(txID, data); } }