Skip to content

Commit

Permalink
feat: increase max L2 to L1 msgs (#6959)
Browse files Browse the repository at this point in the history
See [this doc ](https://hackmd.io/@aztec-network/rJIbJVv40) for more
info - this PR implements the second solution, using variable height
subtrees for L2 to L1 msgs.

### Main changes

- The `out_hash` computed in `base` is now the root of a variable height
merkle tree. The height is calculated from the number of non-empty
messages (this is calculated by moving backwards from the final possible
msg, as we currently don't sort L2 to L1 messages in the kernel). The
variable trees are not cheaper in the circuit because we cannot branch
off hashes we don't need, but the result is smaller trees to deal with
in ts/sol. We can also simply move to a full tree which can hold the new
maximum.
- Because of the variable heights, we cannot supply a `height` to
`Outbox.sol`. Instead, I've added a `minHeight` (height in block + 1)
and check that we haven't reached the maximum height (`minHeight` + 3*,
since I've currently set the max msgs to 8*).
-- NB: I don't believe this exposes much risk, because the usual path
shortening attack works by a user supplying some intermediate node as a
leaf. A user cannot do this here as we must supply a raw message, hash
it in the contract, then check membership. AFAIK it would be too
difficult to find a message preimage for an intermediate node to pass
the `consume` check without a valid message.
- We extract the path in `server.ts` by concatenating the variable
height subtree path and the path created by the rollup circuits hashing
two `out_hash`es. This will *not* work if/when we decide to have 'wonky'
rollups (but in this case, a lot of other aspects will also break e.g.
`txsEffects`, `inHash`)

Note this also fixes the issue with mismatched trees when more than one
transaction in a block has fewer than the max messages (see doc for more
info).
  • Loading branch information
MirandaWood authored Jun 11, 2024
1 parent 5647571 commit 875fb2d
Show file tree
Hide file tree
Showing 25 changed files with 781 additions and 131 deletions.
29 changes: 23 additions & 6 deletions docs/docs/protocol-specs/rollup-circuits/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -507,18 +507,26 @@ graph BT
R[OutHash]
M0[Hash 0-1]
M1[Hash 2-3]
B0[Hash 0.0-0.1]
B0[Hash 0.0-0.3]
B1[Hash 1.0-1.1]
B2[Hash 2.0-2.1]
B3[Hash 3.0-3.1]
K0[l2_to_l1_msgs 0.0]
K1[l2_to_l1_msgs 0.1]
B3[Hash 3.0-3.3]
K0[l2_to_l1_msgs 0.0-0.1]
K1[l2_to_l1_msgs 0.2-0.3]
K2[l2_to_l1_msgs 1.0]
K3[l2_to_l1_msgs 1.1]
K4[l2_to_l1_msgs 2.0]
K5[l2_to_l1_msgs 2.1]
K6[l2_to_l1_msgs 3.0]
K7[l2_to_l1_msgs 3.1]
K6[l2_to_l1_msgs 3.0-3.1]
K7[l2_to_l1_msgs 3.2-3.3]
K8[l2_to_l1_msgs 0.0]
K9[l2_to_l1_msgs 0.1]
K10[l2_to_l1_msgs 0.2]
K11[l2_to_l1_msgs 0.3]
K12[l2_to_l1_msgs 3.0]
K13[l2_to_l1_msgs 3.1]
K14[l2_to_l1_msgs 3.2]
K15[l2_to_l1_msgs 3.3]
M0 --> R
M1 --> R
Expand All @@ -534,6 +542,14 @@ graph BT
K5 --> B2
K6 --> B3
K7 --> B3
K8 --> K0
K9 --> K0
K10 --> K1
K11 --> K1
K12 --> K6
K13 --> K6
K14 --> K7
K15 --> K7
```

```mermaid
Expand Down Expand Up @@ -572,6 +588,7 @@ graph BT

While the `TxsHash` merely require the data to be published and known to L1, the `InHash` and `OutHash` needs to be computable on L1 as well.
This reason require them to be efficiently computable on L1 while still being non-horrible inside a snark - leading us to rely on SHA256.
The L2 to L1 messages from each transaction form a variable height tree. In the diagram above, transactions 0 and 3 have four messages, so require a tree with two layers, whereas the others only have two messages and so require a single layer tree. The base rollup calculates the root of this tree and passes it as the to the next layer. Merge rollups simply hash both of these roots together and pass it up as the `OutHash`.

## Next Steps

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Inserts the root of a merkle tree containing all of the L2 to L1 messages in a b
| -------------- | ------- | ----------- |
| `_l2BlockNumber` | `uint256` | The L2 Block Number in which the L2 to L1 messages reside |
| `_root` | `bytes32` | The merkle root of the tree where all the L2 to L1 messages are leaves |
| `_height` | `uint256` | The height of the merkle tree that the root corresponds to |
| `_minHeight` | `uint256` | The minimum height of the merkle tree that the root corresponds to |

#### Edge cases

Expand Down Expand Up @@ -45,7 +45,7 @@ Allows a recipient to consume a message from the `Outbox`.
- Will revert with `Outbox__InvalidChainId()` if `block.chainid != _message.recipient.chainId`.
- Will revert with `Outbox__NothingToConsumeAtBlock(uint256 l2BlockNumber)` if the root for the block has not been set yet.
- Will revert with `Outbox__AlreadyNullified(uint256 l2BlockNumber, uint256 leafIndex)` if the message at leafIndex for the block has already been consumed.
- Will revert with `Outbox__InvalidPathLength(uint256 expected, uint256 actual)` if the existing height of the L2 to L1 message tree, and the supplied height do not match.
- Will revert with `Outbox__InvalidPathLength(uint256 expected, uint256 actual)` if the the supplied height is less than the existing minimum height of the L2 to L1 message tree, or the supplied height is greater than the maximum (minimum height + log2(maximum messages)).
- Will revert with `MerkleLib__InvalidRoot(bytes32 expected, bytes32 actual, bytes32 leaf, uint256 leafIndex)` if unable to verify the message existence in the tree. It returns the message as a leaf, as well as the index of the leaf to expose more info about the error.


Expand Down
9 changes: 5 additions & 4 deletions l1-contracts/src/core/Rollup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,12 @@ contract Rollup is IRollup {
revert Errors.Rollup__InvalidInHash(inHash, header.contentCommitment.inHash);
}

// We assume here that the number of L2 to L1 messages per tx is 2. Therefore we just need a tree that is one height
// larger (as we can just extend the tree one layer down to hold all the L2 to L1 messages)
uint256 l2ToL1TreeHeight = header.contentCommitment.txTreeHeight + 1;
// Currently trying out storing each tx's L2 to L1 messages in variable height trees (smallest tree required)
// => path lengths will differ and we cannot provide one here
// We can provide a minimum which is the height of the rollup layers (txTreeHeight) and the smallest 'tree' (1 layer)
uint256 l2ToL1TreeMinHeight = header.contentCommitment.txTreeHeight + 1;
OUTBOX.insert(
header.globalVariables.blockNumber, header.contentCommitment.outHash, l2ToL1TreeHeight
header.globalVariables.blockNumber, header.contentCommitment.outHash, l2ToL1TreeMinHeight
);

// pay the coinbase 1 gas token if it is not empty and header.totalFees is not zero
Expand Down
4 changes: 2 additions & 2 deletions l1-contracts/src/core/interfaces/messagebridge/IOutbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ interface IOutbox {
* @dev Emits `RootAdded` upon inserting the root successfully
* @param _l2BlockNumber - The L2 Block Number in which the L2 to L1 messages reside
* @param _root - The merkle root of the tree where all the L2 to L1 messages are leaves
* @param _height - The height of the merkle tree that the root corresponds to
* @param _minHeight - The min height of the merkle tree that the root corresponds to
*/
function insert(uint256 _l2BlockNumber, bytes32 _root, uint256 _height) external;
function insert(uint256 _l2BlockNumber, bytes32 _root, uint256 _minHeight) external;
// docs:end:outbox_insert

// docs:start:outbox_consume
Expand Down
16 changes: 8 additions & 8 deletions l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ library Constants {
uint256 internal constant PROTOCOL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX = 1;
uint256 internal constant MAX_TOTAL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX = 32;
uint256 internal constant MAX_PUBLIC_DATA_READS_PER_TX = 32;
uint256 internal constant MAX_NEW_L2_TO_L1_MSGS_PER_TX = 2;
uint256 internal constant MAX_NEW_L2_TO_L1_MSGS_PER_TX = 8;
uint256 internal constant MAX_NOTE_HASH_READ_REQUESTS_PER_TX = 128;
uint256 internal constant MAX_NULLIFIER_READ_REQUESTS_PER_TX = 128;
uint256 internal constant MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_TX = 128;
Expand Down Expand Up @@ -156,14 +156,14 @@ library Constants {
uint256 internal constant PUBLIC_DATA_READ_LENGTH = 2;
uint256 internal constant VALIDATION_REQUESTS_LENGTH = 1538;
uint256 internal constant PUBLIC_DATA_UPDATE_REQUEST_LENGTH = 3;
uint256 internal constant COMBINED_ACCUMULATED_DATA_LENGTH = 231;
uint256 internal constant COMBINED_ACCUMULATED_DATA_LENGTH = 237;
uint256 internal constant COMBINED_CONSTANT_DATA_LENGTH = 40;
uint256 internal constant CALL_REQUEST_LENGTH = 7;
uint256 internal constant PRIVATE_ACCUMULATED_DATA_LENGTH = 1128;
uint256 internal constant PRIVATE_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 2715;
uint256 internal constant PUBLIC_ACCUMULATED_DATA_LENGTH = 881;
uint256 internal constant PUBLIC_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 3566;
uint256 internal constant KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 281;
uint256 internal constant PRIVATE_ACCUMULATED_DATA_LENGTH = 1152;
uint256 internal constant PRIVATE_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 2739;
uint256 internal constant PUBLIC_ACCUMULATED_DATA_LENGTH = 887;
uint256 internal constant PUBLIC_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 3578;
uint256 internal constant KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 287;
uint256 internal constant CONSTANT_ROLLUP_DATA_LENGTH = 14;
uint256 internal constant BASE_OR_MERGE_PUBLIC_INPUTS_LENGTH = 31;
uint256 internal constant ENQUEUE_PUBLIC_FUNCTION_CALL_RETURN_LENGTH = 9;
Expand All @@ -174,7 +174,7 @@ library Constants {
uint256 internal constant CONTRACTS_NUM_BYTES_PER_BASE_ROLLUP = 32;
uint256 internal constant CONTRACT_DATA_NUM_BYTES_PER_BASE_ROLLUP = 64;
uint256 internal constant CONTRACT_DATA_NUM_BYTES_PER_BASE_ROLLUP_UNPADDED = 52;
uint256 internal constant L2_TO_L1_MSGS_NUM_BYTES_PER_BASE_ROLLUP = 64;
uint256 internal constant L2_TO_L1_MSGS_NUM_BYTES_PER_BASE_ROLLUP = 256;
uint256 internal constant LOGS_HASHES_NUM_BYTES_PER_BASE_ROLLUP = 64;
uint256 internal constant NUM_MSGS_PER_BASE_PARITY = 4;
uint256 internal constant NUM_BASE_PARITY_PER_ROOT_PARITY = 4;
Expand Down
2 changes: 1 addition & 1 deletion l1-contracts/src/core/libraries/decoders/TxsDecoder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ library TxsDecoder {
// We pad base leaves with hashes of empty tx effect.
for (uint256 i = numTxEffects; i < vars.baseLeaves.length; i++) {
// Value taken from tx_effect.test.ts "hash of empty tx effect matches snapshot" test case
vars.baseLeaves[i] = hex"003f2c7d671d4a2c210124550cf00f8e21727a0ae1a43e1758982a25725dde2b";
vars.baseLeaves[i] = hex"0016cc39e093d21650607a4fe4ccbbb56b1219575378edea7fbe80a96e909603";
}
}

Expand Down
43 changes: 35 additions & 8 deletions l1-contracts/src/core/messagebridge/Outbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pragma solidity >=0.8.18;
// Libraries
import {DataStructures} from "../libraries/DataStructures.sol";
import {Errors} from "../libraries/Errors.sol";
import {Constants} from "../libraries/ConstantsGen.sol";
import {MerkleLib} from "../libraries/MerkleLib.sol";
import {Hash} from "../libraries/Hash.sol";
import {IOutbox} from "../interfaces/messagebridge/IOutbox.sol";
Expand All @@ -21,7 +22,7 @@ contract Outbox is IOutbox {
struct RootData {
// This is the outhash specified by header.globalvariables.outHash of any given block.
bytes32 root;
uint256 height;
uint256 minHeight;
mapping(uint256 => bool) nullified;
}

Expand All @@ -39,9 +40,9 @@ contract Outbox is IOutbox {
* @dev Emits `RootAdded` upon inserting the root successfully
* @param _l2BlockNumber - The L2 Block Number in which the L2 to L1 messages reside
* @param _root - The merkle root of the tree where all the L2 to L1 messages are leaves
* @param _height - The height of the merkle tree that the root corresponds to
* @param _minHeight - The min height of the merkle tree that the root corresponds to
*/
function insert(uint256 _l2BlockNumber, bytes32 _root, uint256 _height)
function insert(uint256 _l2BlockNumber, bytes32 _root, uint256 _minHeight)
external
override(IOutbox)
{
Expand All @@ -58,9 +59,9 @@ contract Outbox is IOutbox {
}

roots[_l2BlockNumber].root = _root;
roots[_l2BlockNumber].height = _height;
roots[_l2BlockNumber].minHeight = _minHeight;

emit RootAdded(_l2BlockNumber, _root, _height);
emit RootAdded(_l2BlockNumber, _root, _minHeight);
}

/**
Expand Down Expand Up @@ -100,12 +101,20 @@ contract Outbox is IOutbox {
revert Errors.Outbox__AlreadyNullified(_l2BlockNumber, _leafIndex);
}

uint256 treeHeight = rootData.height;

if (treeHeight != _path.length) {
// Min height = height of rollup layers
// The smallest num of messages will require a subtree of height 1
uint256 treeHeight = rootData.minHeight;
if (treeHeight > _path.length) {
revert Errors.Outbox__InvalidPathLength(treeHeight, _path.length);
}

// Max height = height of rollup layers + max possible subtree height
// The max num of messages N will require a subtree of height log2(N)
uint256 maxSubtreeHeight = calculateTreeHeightFromSize(Constants.MAX_NEW_L2_TO_L1_MSGS_PER_TX);
if (treeHeight + maxSubtreeHeight < _path.length) {
revert Errors.Outbox__InvalidPathLength(treeHeight + maxSubtreeHeight, _path.length);
}

bytes32 messageHash = _message.sha256ToField();

MerkleLib.verifyMembership(_path, messageHash, _leafIndex, blockRoot);
Expand All @@ -129,4 +138,22 @@ contract Outbox is IOutbox {
{
return roots[_l2BlockNumber].nullified[_leafIndex];
}

/**
* @notice Calculates a tree height from the amount of elements in the tree
* @dev - This mirrors the function in TestUtil, but assumes _size is an exact power of 2
* @param _size - The number of elements in the tree
*/
function calculateTreeHeightFromSize(uint256 _size) internal pure returns (uint256) {
/// We need the height of the tree that will contain all of our leaves,
/// hence the next highest power of two from the amount of leaves - Math.ceil(Math.log2(x))
uint256 height = 0;

/// While size > 1, we divide by two, and count how many times we do this; producing a rudimentary way of calculating Math.Floor(Math.log2(x))
while (_size > 1) {
_size >>= 1;
height++;
}
return height;
}
}
6 changes: 3 additions & 3 deletions l1-contracts/test/Outbox.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,13 @@ contract OutboxTest is Test {
vm.prank(ROLLUP_CONTRACT);
outbox.insert(1, root, DEFAULT_TREE_HEIGHT);

NaiveMerkle biggerTree = new NaiveMerkle(DEFAULT_TREE_HEIGHT + 1);
NaiveMerkle smallerTree = new NaiveMerkle(DEFAULT_TREE_HEIGHT - 1);
tree.insertLeaf(leaf);

(bytes32[] memory path,) = biggerTree.computeSiblingPath(0);
(bytes32[] memory path,) = smallerTree.computeSiblingPath(0);
vm.expectRevert(
abi.encodeWithSelector(
Errors.Outbox__InvalidPathLength.selector, DEFAULT_TREE_HEIGHT, DEFAULT_TREE_HEIGHT + 1
Errors.Outbox__InvalidPathLength.selector, DEFAULT_TREE_HEIGHT, DEFAULT_TREE_HEIGHT - 1
)
);
outbox.consume(fakeMessage, 1, 0, path);
Expand Down
12 changes: 11 additions & 1 deletion l1-contracts/test/Rollup.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,20 @@ contract RollupTest is DecoderBase {

bytes32 l2ToL1MessageTreeRoot;
{
// NB: The below works with full blocks because we require the largest possible subtrees
// for L2 to L1 messages - usually we make variable height subtrees, the roots of which
// form a balanced tree
uint256 numTxsWithPadding = txsHelper.computeNumTxEffectsToPad(numTxs) + numTxs;
// The below is a little janky - we know that this test deals with full txs with equal numbers
// of msgs or txs with no messages, so the division works
// TODO edit full.messages to include information about msgs per tx?
uint256 subTreeHeight = merkleTestUtil.calculateTreeHeightFromSize(
full.messages.l2ToL1Messages.length == 0 ? 0 : full.messages.l2ToL1Messages.length / numTxs
);
uint256 outHashTreeHeight = merkleTestUtil.calculateTreeHeightFromSize(numTxsWithPadding);
uint256 numMessagesWithPadding = numTxsWithPadding * Constants.MAX_NEW_L2_TO_L1_MSGS_PER_TX;

uint256 treeHeight = merkleTestUtil.calculateTreeHeightFromSize(numMessagesWithPadding);
uint256 treeHeight = subTreeHeight + outHashTreeHeight;
NaiveMerkle tree = new NaiveMerkle(treeHeight);
for (uint256 i = 0; i < numMessagesWithPadding; i++) {
if (i < full.messages.l2ToL1Messages.length) {
Expand Down
14 changes: 7 additions & 7 deletions l1-contracts/test/fixtures/empty_block_0.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,23 @@
"l2ToL1Messages": []
},
"block": {
"archive": "0x0ca6026d7140e6c6796115b226b84254cf06fcc7d57e02eee6f7a141f37f79a2",
"archive": "0x2cc5c86423dfddcb0217275afdef30cc159a1844bbf04c4e7f2726bda25a6c82",
"body": "0x00000000",
"txsEffectsHash": "0x008b39a331f763fd73b42ee12d3ed687ad7cf05751869d4bf875eda18c1c94d9",
"txsEffectsHash": "0x00f063d3d04bdfdbb13c70a1733363ef91abb4e69976167b3241ac2d3f18042f",
"decodedHeader": {
"contentCommitment": {
"inHash": "0x00089a9d421a82c4a25f7acbebe69e638d5b064fa8a60e018793dcb0be53752c",
"outHash": "0x0007638bb56b6dda2b64b8f76841114ac3a87a1820030e2e16772c4d294879c3",
"txTreeHeight": 1,
"txsEffectsHash": "0x008b39a331f763fd73b42ee12d3ed687ad7cf05751869d4bf875eda18c1c94d9"
"txsEffectsHash": "0x00f063d3d04bdfdbb13c70a1733363ef91abb4e69976167b3241ac2d3f18042f"
},
"globalVariables": {
"blockNumber": 1,
"chainId": 31337,
"timestamp": 0,
"version": 1,
"coinbase": "0x4bcfac0542d733e71b510c1907a85d283f02bbd7",
"feeRecipient": "0x052322027092ecfaeb88d75a31d553f791886da288ccedc3c642899c8baf4c6a",
"coinbase": "0x8fec2ecdef22c902cdf174c3a4b834e9cbe5a823",
"feeRecipient": "0x2c72c2766e74fa90a1613622187a3420b6458bed6600ea4d1f2cb76de1b053a1",
"gasFees": {
"feePerDaGas": 0,
"feePerL2Gas": 0
Expand Down Expand Up @@ -55,8 +55,8 @@
}
}
},
"header": "0x05b0b6df52f1d47d0406318558052c89a174fbc9d615def82b3cc9ccc1937db8000000010000000000000000000000000000000000000000000000000000000000000001008b39a331f763fd73b42ee12d3ed687ad7cf05751869d4bf875eda18c1c94d900089a9d421a82c4a25f7acbebe69e638d5b064fa8a60e018793dcb0be53752c0007638bb56b6dda2b64b8f76841114ac3a87a1820030e2e16772c4d294879c31864fcdaa80ff2719154fa7c8a9050662972707168d69eac9db6fd3110829f800000001016642d9ccd8346c403aa4c3fa451178b22534a27035cdaa6ec34ae53b29c50cb000000800bcfa3e9f1a8922ee92c6dc964d6595907c1804a86753774322b468f69d4f278000001000572c8db882674dd026b8877fbba1b700a4407da3ae9ce5fa43215a28163362b000000800000000000000000000000000000000000000000000000000000000000007a690000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000004bcfac0542d733e71b510c1907a85d283f02bbd7052322027092ecfaeb88d75a31d553f791886da288ccedc3c642899c8baf4c6a000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
"publicInputsHash": "0x00a63e965b18f145af4be3954bf6918a484c8c02dcd3892e665fb4b0e74f9f39",
"header": "0x05b0b6df52f1d47d0406318558052c89a174fbc9d615def82b3cc9ccc1937db800000001000000000000000000000000000000000000000000000000000000000000000100f063d3d04bdfdbb13c70a1733363ef91abb4e69976167b3241ac2d3f18042f00089a9d421a82c4a25f7acbebe69e638d5b064fa8a60e018793dcb0be53752c0007638bb56b6dda2b64b8f76841114ac3a87a1820030e2e16772c4d294879c31864fcdaa80ff2719154fa7c8a9050662972707168d69eac9db6fd3110829f800000001016642d9ccd8346c403aa4c3fa451178b22534a27035cdaa6ec34ae53b29c50cb000000800bcfa3e9f1a8922ee92c6dc964d6595907c1804a86753774322b468f69d4f278000001000572c8db882674dd026b8877fbba1b700a4407da3ae9ce5fa43215a28163362b000000800000000000000000000000000000000000000000000000000000000000007a690000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000008fec2ecdef22c902cdf174c3a4b834e9cbe5a8232c72c2766e74fa90a1613622187a3420b6458bed6600ea4d1f2cb76de1b053a1000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
"publicInputsHash": "0x000478e846843b104663b22acbcca1213ac479af56bbb59afde93b5c1aff96ca",
"numTxs": 0
}
}
Loading

0 comments on commit 875fb2d

Please sign in to comment.