diff --git a/contracts/CHANGELOG.md b/contracts/CHANGELOG.md index 25a91f28..529d7464 100644 --- a/contracts/CHANGELOG.md +++ b/contracts/CHANGELOG.md @@ -6,6 +6,12 @@ The format is inspired by [Keep a Changelog]. [Keep a Changelog]: https://keepachangelog.com/en/1.0.0/ +## 0.2.9 (2024-07-01) + +### Fixed + + * CBOR unsigned integer decoding in Subcall.sol + ## 0.2.8 (2024-03-15) ### Fixed diff --git a/contracts/contracts/Subcall.sol b/contracts/contracts/Subcall.sol index a87f2a77..609322a5 100644 --- a/contracts/contracts/Subcall.sol +++ b/contracts/contracts/Subcall.sol @@ -52,7 +52,7 @@ library Subcall { error InvalidMap(); /// While parsing CBOR structure, data length was unexpected - error InvalidLength(); + error InvalidLength(uint256); /// Invalid receipt ID error InvalidReceiptId(); @@ -63,6 +63,12 @@ library Subcall { /// CBOR parser expected a key, but it was not found in the map! error MissingKey(); + /// Value cannot be parsed as a uint + error InvalidUintPrefix(uint8); + + /// Unsigned integer of unknown size + error InvalidUintSize(uint8); + /** * @notice Submit a native message to the Oasis runtime layer. Messages * which re-enter the EVM module are forbidden: `evm.*`. @@ -170,6 +176,11 @@ library Subcall { (uint64, bytes) ); + // 0xf6 = null, returns null in case receiptId not found + if (result[0] == 0xf6) { + revert InvalidReceiptId(); + } + if (status != 0) { revert ConsensusTakeReceiptError(status, string(result)); } @@ -182,11 +193,42 @@ library Subcall { pure returns (uint256 newOffset, uint256 value) { - if (result[offset] & 0x40 != 0x40) revert InvalidLength(); + uint8 prefix = uint8(result[offset]); + uint256 len; - uint256 len = uint8(result[offset++]) ^ 0x40; + if (prefix <= 0x17) { + return (offset + 1, prefix); + } + // Byte array(uint256), parsed as a big-endian integer. + else if (prefix == 0x58) { + len = uint8(result[++offset]); + offset++; + } + // Byte array, parsed as a big-endian integer. + else if (prefix & 0x40 == 0x40) { + len = uint8(result[offset++]) ^ 0x40; + } + // Unsigned integer, CBOR encoded. + else if (prefix & 0x10 == 0x10) { + if (prefix == 0x18) { + len = 1; + } else if (prefix == 0x19) { + len = 2; + } else if (prefix == 0x1a) { + len = 4; + } else if (prefix == 0x1b) { + len = 8; + } else { + revert InvalidUintSize(prefix); + } + offset += 1; + } + // Unknown... + else { + revert InvalidUintPrefix(prefix); + } - if (len >= 0x20) revert InvalidLength(); + if (len > 0x20) revert InvalidLength(len); assembly { value := mload(add(add(0x20, result), offset)) @@ -267,9 +309,6 @@ library Subcall { (offset, endReceipt) = _parseCBORUint64(result, offset); hasReceipt = true; - } else { - // TODO: skip unknown keys & values? For forward compatibility - revert InvalidKey(); } } @@ -296,9 +335,6 @@ library Subcall { (offset, amount) = _parseCBORUint128(result, offset); hasAmount = true; - } else { - // TODO: skip unknown keys & values? For forward compatibility - revert InvalidKey(); } } @@ -321,7 +357,7 @@ library Subcall { // Delegation succeeded, decode number of shares. uint8 sharesLen = uint8(result[8]) & 0x1f; // Assume shares field is never greater than 16 bytes. - if (9 + sharesLen != result.length) revert InvalidLength(); + if (9 + sharesLen != result.length) revert InvalidLength(sharesLen); for (uint256 offset = 0; offset < sharesLen; offset++) { uint8 v = uint8(result[9 + offset]); diff --git a/contracts/contracts/tests/SubcallTests.sol b/contracts/contracts/tests/SubcallTests.sol index 98377142..6e6d649d 100644 --- a/contracts/contracts/tests/SubcallTests.sol +++ b/contracts/contracts/tests/SubcallTests.sol @@ -108,4 +108,12 @@ contract SubcallTests { function testConsensusWithdraw(StakingAddress to, uint128 value) external { Subcall.consensusWithdraw(to, value); } + + function testParseCBORUint(bytes memory result, uint256 offset) + external + pure + returns (uint256, uint256) + { + return Subcall._parseCBORUint(result, offset); + } } diff --git a/contracts/package.json b/contracts/package.json index c29ed6eb..86df039b 100644 --- a/contracts/package.json +++ b/contracts/package.json @@ -1,6 +1,6 @@ { "name": "@oasisprotocol/sapphire-contracts", - "version": "0.2.8", + "version": "0.2.9", "license": "Apache-2.0", "description": "Solidity smart contract library for confidential contract development", "homepage": "https://github.com/oasisprotocol/sapphire-paratime/tree/main/contracts", diff --git a/contracts/test/subcall.ts b/contracts/test/subcall.ts index 03ee51ee..76dd38be 100644 --- a/contracts/test/subcall.ts +++ b/contracts/test/subcall.ts @@ -78,7 +78,9 @@ async function ensureBalance( await resp.wait(); } const newBalance = await provider.getBalance(address); - expect(newBalance).eq(initialBalance); + expect(newBalance).gte(initialBalance); + + return newBalance; } function decodeResult(receipt: ContractTransactionReceipt) { @@ -104,7 +106,13 @@ describe('Subcall', () => { let provider: Provider; before(async () => { - const factory = await ethers.getContractFactory('SubcallTests'); + const subcallFactory = await ethers.getContractFactory('Subcall'); + const subcallLib = await subcallFactory.deploy(); + await subcallLib.waitForDeployment(); + + const factory = await ethers.getContractFactory('SubcallTests', { + libraries: { Subcall: await subcallLib.getAddress() }, + }); contract = (await factory.deploy({ value: parseEther('1.0'), })) as unknown as SubcallTests; @@ -240,8 +248,7 @@ describe('Subcall', () => { ); // Ensure contract has an initial balance, above minimum delegation amount - const initialBalance = parseEther('100'); - await ensureBalance(contract, initialBalance, owner); + await ensureBalance(contract, parseEther('100'), owner); // Perform delegation, and request a receipt let receiptId = randomInt(2 ** 32, 2 ** 32 * 2); @@ -283,17 +290,29 @@ describe('Subcall', () => { // Retrieve UndelegateStart receipt tx = await contract.testTakeReceipt(2, nextReceiptId); receipt = await tx.wait(); - result = cborg.decode(getBytes((receipt?.logs![0] as EventLog).args!.data)); + let resultBytes = (receipt?.logs![0] as EventLog).args!.data; + result = cborg.decode(getBytes(resultBytes)); expect(result.receipt).eq(nextReceiptId); + // Try decoding undelegate start receipt + const undelegateDecoded = await contract.testDecodeReceiptUndelegateStart( + resultBytes, + ); + expect(undelegateDecoded[1]).eq(result.receipt); + + const initialContractBalance = await ethers.provider.getBalance( + await contract.getAddress(), + ); + await dockerSkipEpochs({ targetEpoch: result.epoch }); - // Retrieve UndelegateStart receipt + // Retrieve UndelegateDone receipt tx = await contract.testTakeReceipt(3, result.receipt); receipt = await tx.wait(); - result = cborg.decode(getBytes((receipt?.logs![0] as EventLog).args!.data)); + resultBytes = (receipt?.logs![0] as EventLog).args!.data; + result = cborg.decode(getBytes(resultBytes)); expect(await ethers.provider.getBalance(await contract.getAddress())).eq( - parseEther('100'), + initialContractBalance + parseEther('100'), ); }); @@ -350,4 +369,106 @@ describe('Subcall', () => { expect(shares).eq(toBigInt(num)); } }); + + describe('Should successfully parse CBOR uint/s', () => { + it('Should successfully parse CBOR uint8', async () => { + const MAX_SAFE_UINT8 = 255n; + + // bytes = 0x18FF + const bytes = cborg.encode(MAX_SAFE_UINT8); + + const [newOffset, parsedCborUint] = await contract.testParseCBORUint( + bytes, + 0, + ); + + expect(parsedCborUint).eq(MAX_SAFE_UINT8); + expect(newOffset).eq(1 + 1); + }); + + it('Should successfully parse CBOR uint16', async () => { + const MAX_SAFE_UINT16 = 65535n; + + // bytes = 0x19FFFF + const bytes = cborg.encode(MAX_SAFE_UINT16); + + const [newOffset, parsedCborUint] = await contract.testParseCBORUint( + bytes, + 0, + ); + + expect(parsedCborUint).eq(MAX_SAFE_UINT16); + expect(newOffset).eq(2 + 1); + }); + + it('Should successfully parse CBOR uint32', async () => { + const MAX_SAFE_UINT32 = 4294967295n; + + // bytes = 0x1AFFFFFFFF + const bytes = cborg.encode(MAX_SAFE_UINT32); + + const [newOffset, parsedCborUint] = await contract.testParseCBORUint( + bytes, + 0, + ); + + expect(parsedCborUint).eq(MAX_SAFE_UINT32); + expect(newOffset).eq(4 + 1); + }); + + it('Should successfully parse CBOR uint64', async () => { + const MAX_SAFE_UINT64 = 18446744073709551615n; + + // bytes = 0x1BFFFFFFFFFFFFFFFF + const bytes = cborg.encode(MAX_SAFE_UINT64); + + const [newOffset, parsedCborUint] = await contract.testParseCBORUint( + bytes, + 0, + ); + + expect(parsedCborUint).eq(MAX_SAFE_UINT64); + expect(newOffset).eq(8 + 1); + }); + + it('Should successfully parse CBOR uint128', async () => { + const MAX_SAFE_UINT128 = 340282366920938463463374607431768211455n; + + const hex = '0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF'; + const uint128bytes = Uint8Array.from( + Buffer.from(hex.replace('0x', ''), 'hex'), + ); + + const bytes = cborg.encode(uint128bytes); + + const [newOffset, parsedCborUint] = await contract.testParseCBORUint( + bytes, + 0, + ); + + expect(parsedCborUint).eq(MAX_SAFE_UINT128); + expect(newOffset).eq(16 + 1); + }); + + it('Should successfully parse CBOR uint256', async () => { + const MAX_SAFE_UINT256 = + 115792089237316195423570985008687907853269984665640564039457584007913129639935n; + + const hex = + '0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF'; + const uint256bytes = Uint8Array.from( + Buffer.from(hex.replace('0x', ''), 'hex'), + ); + + const bytes = cborg.encode(uint256bytes); + + const [newOffset, parsedCborUint] = await contract.testParseCBORUint( + bytes, + 0, + ); + + expect(parsedCborUint).eq(MAX_SAFE_UINT256); + expect(newOffset).eq(33 + 1); + }); + }); });