Skip to content

Implement IOutputsMerkleRootValidator #133

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

Merged
merged 17 commits into from
Apr 10, 2025
Merged

Implement IOutputsMerkleRootValidator #133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
0034264
feat(consensus): update rollups contracts to v2.0.0-rc.17
GCdePaula Apr 7, 2025
1f118f9
feat(consensus): add soldeer, implement ERC165
GCdePaula Apr 7, 2025
6b49b04
fix(consensus): fix "local" dependencies include
GCdePaula Apr 7, 2025
bc53920
feat(consensus): use soldeer for rollups contracts
GCdePaula Apr 7, 2025
873dbb2
feat: add soldeer install to setup
GCdePaula Apr 7, 2025
f039e1e
feat(consensus): update solidity-step
GCdePaula Apr 7, 2025
a8a8ff1
feat(consensus): implement `IOutputsMerkleRootValidator`
GCdePaula Apr 7, 2025
fd20219
chore(consensus): add remappings.txt to .gitignore
GCdePaula Apr 7, 2025
b82f883
refactor(consensus): improve merkle tree size check
GCdePaula Apr 7, 2025
ec14593
chore(step): update solidity step version
GCdePaula Apr 8, 2025
a90c3a3
refactor(consensus): address review comments
GCdePaula Apr 8, 2025
be852a9
feat(node): add output merkle and proof for `settle`
stephenctw Apr 8, 2025
634e83d
fix(consensus): fix output merkle, refactor machine runner
GCdePaula Apr 9, 2025
4de2d96
chore: minor code cleanup
stephenctw Apr 9, 2025
cb0a068
refactor(consensus): improve tests
GCdePaula Apr 9, 2025
324279f
feat(client-lua): update offchain `EpochSealed` event
stephenctw Apr 9, 2025
59993ae
refactor(consensus): use checks-effects-interactions pattern
GCdePaula Apr 10, 2025
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
9 changes: 0 additions & 9 deletions .gitmodules
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,6 @@
[submodule "machine/emulator"]
path = machine/emulator
url = https://github.com/cartesi/machine-emulator
[submodule "cartesi-rollups/contracts/lib/forge-std"]
path = cartesi-rollups/contracts/lib/forge-std
url = https://github.com/foundry-rs/forge-std
[submodule "cartesi-rollups/contracts/lib/rollups-contracts"]
path = cartesi-rollups/contracts/lib/rollups-contracts
url = https://github.com/cartesi/rollups-contracts
[submodule "cartesi-rollups/contracts/lib/openzeppelin-contracts"]
path = cartesi-rollups/contracts/lib/openzeppelin-contracts
url = https://github.com/openzeppelin/openzeppelin-contracts
[submodule "test/programs/honeypot/honeypot"]
path = test/programs/honeypot/honeypot
url = https://github.com/cartesi/honeypot
5 changes: 5 additions & 0 deletions cartesi-rollups/contracts/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
cache/
out/


# soldeer
/dependencies
remappings.txt

# Ignores development broadcast logs
!/broadcast
/broadcast/*/31337/
Expand Down
3 changes: 3 additions & 0 deletions cartesi-rollups/contracts/.soldeerignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package.json
pnpm-lock.yaml
test
18 changes: 10 additions & 8 deletions cartesi-rollups/contracts/foundry.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
[profile.default]
src = "src"
out = "out"
libs = ["lib"]
libs = ["dependencies", "../../prt/contracts", "../../machine/step"]
via_ir = true

remappings = [
'openzeppelin-contracts/=lib/openzeppelin-contracts/contracts/',
'rollups-contracts/=lib/rollups-contracts/contracts/',
'@openzeppelin-contracts-5.2.0/=dependencies/@openzeppelin-contracts-5.2.0/',
'forge-std-1.9.6/=dependencies/forge-std-1.9.6/',
'cartesi-rollups-contracts-2.0.0/=dependencies/cartesi-rollups-contracts-2.0.0-rc.17/src/',

'prt-contracts/=../../prt/contracts/src/',
'step/=../../machine/step/',
]

allow_paths = [
'../../prt/contracts/src/',
'../../machine/step/',
]

solc-version = "0.8.27"

[dependencies]
"@openzeppelin-contracts" = "5.2.0"
forge-std = "1.9.6"
cartesi-rollups-contracts = "2.0.0-rc.17"
3 changes: 3 additions & 0 deletions cartesi-rollups/contracts/justfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ fmt:
check-fmt:
forge fmt --check

install-deps:
forge soldeer install

# compile smart contracts
build-smart-contracts:
forge build
Expand Down
1 change: 0 additions & 1 deletion cartesi-rollups/contracts/lib/forge-std
Open in desktop
Submodule forge-std deleted from 978ac6
1 change: 0 additions & 1 deletion cartesi-rollups/contracts/lib/openzeppelin-contracts
Open in desktop
Submodule openzeppelin-contracts deleted from acd4ff
1 change: 0 additions & 1 deletion cartesi-rollups/contracts/lib/rollups-contracts
Open in desktop
Submodule rollups-contracts deleted from 913358
7 changes: 4 additions & 3 deletions cartesi-rollups/contracts/script/DaveConsensus.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ pragma solidity ^0.8.17;
import {Script} from "forge-std/Script.sol";

import {Machine} from "prt-contracts/types/Machine.sol";

import "prt-contracts/tournament/factories/MultiLevelTournamentFactory.sol";
import "prt-contracts/../test/constants/TestTournamentParametersProvider.sol";
import "prt-contracts/state-transition/CmioStateTransition.sol";
import "prt-contracts/state-transition/RiscVStateTransition.sol";
import "prt-contracts/state-transition/CartesiStateTransition.sol";
import "rollups-contracts/inputs/IInputBox.sol";
import "prt-contracts/../test/constants/TestTournamentParametersProvider.sol";

import "cartesi-rollups-contracts-2.0.0/inputs/IInputBox.sol";

import "src/DaveConsensus.sol";

// Only used for tests
Expand Down
2 changes: 1 addition & 1 deletion cartesi-rollups/contracts/script/InputBox.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pragma solidity ^0.8.17;

import {Script} from "forge-std/Script.sol";

import "rollups-contracts/inputs/InputBox.sol";
import "cartesi-rollups-contracts-2.0.0/inputs/InputBox.sol";

// Only used for tests
contract InputBoxScript is Script {
Expand Down
20 changes: 20 additions & 0 deletions cartesi-rollups/contracts/soldeer.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[[dependencies]]
name = "@openzeppelin-contracts"
version = "5.2.0"
url = "https://soldeer-revisions.s3.amazonaws.com/@openzeppelin-contracts/5_2_0_11-01-2025_09:30:20_contracts.zip"
checksum = "6dbd0440446b2ed16ca25e9f1af08fc0c5c1e73e71fee86ae8a00daa774e3817"
integrity = "4cb7f3777f67fdf4b7d0e2f94d2f93f198b2e5dce718b7062ac7c2c83e1183bd"

[[dependencies]]
name = "cartesi-rollups-contracts"
version = "2.0.0-rc.17"
url = "https://soldeer-revisions.s3.amazonaws.com/cartesi-rollups-contracts/2_0_0-rc_17_28-03-2025_19:26:24_rollups-contracts.zip"
checksum = "72fed984d9d6464041fe038f9740c2ddb7eb478fc3959f050a0605e8d346fe08"
integrity = "6f9cdd1a1eedafe1d71e767492bbad9f6aaf5eec73b3bdd919152b6b65a6aa72"

[[dependencies]]
name = "forge-std"
version = "1.9.6"
url = "https://soldeer-revisions.s3.amazonaws.com/forge-std/1_9_6_01-02-2025_20:49:10_forge-std-1.9.zip"
checksum = "55f341818321b3f925161a72fd0dcd62e4a0a4b66785a7a932bf2bfaf96fb9d1"
integrity = "e9ecdc364d152157431e5df5aa041ffddbe9bb1c1ad81634b1e72df9e23814e8"
103 changes: 88 additions & 15 deletions cartesi-rollups/contracts/src/DaveConsensus.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@

pragma solidity ^0.8.8;

import {IInputBox} from "rollups-contracts/inputs/IInputBox.sol";
import {IERC165} from "@openzeppelin-contracts-5.2.0/utils/introspection/IERC165.sol";
import {ERC165} from "@openzeppelin-contracts-5.2.0/utils/introspection/ERC165.sol";

import {IOutputsMerkleRootValidator} from "cartesi-rollups-contracts-2.0.0/consensus/IOutputsMerkleRootValidator.sol";
import {IInputBox} from "cartesi-rollups-contracts-2.0.0/inputs/IInputBox.sol";
import {LibMerkle32} from "cartesi-rollups-contracts-2.0.0/library/LibMerkle32.sol";

import {IDataProvider} from "prt-contracts/IDataProvider.sol";
import {ITournamentFactory} from "prt-contracts/ITournamentFactory.sol";
Expand All @@ -12,6 +17,9 @@ import {ITournament} from "prt-contracts/ITournament.sol";
import {Machine} from "prt-contracts/types/Machine.sol";
import {Tree} from "prt-contracts/types/Tree.sol";

import {EmulatorConstants} from "step/src/EmulatorConstants.sol";
import {Memory} from "step/src/Memory.sol";

import {Merkle} from "./Merkle.sol";

/// @notice Consensus contract with Dave tournaments.
Expand All @@ -36,8 +44,9 @@ import {Merkle} from "./Merkle.sol";
/// the accumlating epoch will be sealed, and a new
/// accumulating epoch will be created.
///
contract DaveConsensus is IDataProvider {
contract DaveConsensus is IDataProvider, IOutputsMerkleRootValidator, ERC165 {
using Merkle for bytes;
using LibMerkle32 for bytes32[];

/// @notice The input box contract
IInputBox immutable _inputBox;
Expand All @@ -60,6 +69,9 @@ contract DaveConsensus is IDataProvider {
/// @notice Current sealed epoch tournament
ITournament _tournament;

/// @notice Settled output trees' merkle root hash
mapping(bytes32 => bool) _outputsMerkleRoots;

/// @notice Consensus contract was created
/// @param inputBox the input box contract
/// @param appContract the application contract
Expand All @@ -71,12 +83,14 @@ contract DaveConsensus is IDataProvider {
/// @param inputIndexLowerBound the input index (inclusive) lower bound in the sealed epoch
/// @param inputIndexUpperBound the input index (exclusive) upper bound in the sealed epoch
/// @param initialMachineStateHash the initial machine state hash
/// @param outputsMerkleRoot the Merkle root hash of the outputs tree
/// @param tournament the sealed epoch tournament contract
event EpochSealed(
uint256 epochNumber,
uint256 inputIndexLowerBound,
uint256 inputIndexUpperBound,
Machine.Hash initialMachineStateHash,
bytes32 outputsMerkleRoot,
ITournament tournament
);

Expand All @@ -93,6 +107,19 @@ contract DaveConsensus is IDataProvider {
/// @param fromInputBox Hash of input stored on the input box contract
error InputHashMismatch(bytes32 fromReceivedInput, bytes32 fromInputBox);

/// @notice Supplied output tree proof not consistent with settled machine hash
/// @param settledState Settled machine state hash
error InvalidOutputsMerkleRootProof(Machine.Hash settledState);

/// @notice Supplied output tree proof size is incorrect
/// @param suppliedProofSize Supplied proof size
error InvalidOutputsMerkleRootProofSize(uint256 suppliedProofSize);

/// @notice Application address does not match
/// @param expected Expected application address
/// @param received Received application address
error ApplicationMismatch(address expected, address received);

constructor(
IInputBox inputBox,
address appContract,
Expand All @@ -110,30 +137,43 @@ contract DaveConsensus is IDataProvider {
_inputIndexUpperBound = inputIndexUpperBound;
ITournament tournament = tournamentFactory.instantiate(initialMachineStateHash, this);
_tournament = tournament;
emit EpochSealed(0, 0, inputIndexUpperBound, initialMachineStateHash, tournament);
emit EpochSealed(0, 0, inputIndexUpperBound, initialMachineStateHash, bytes32(0), tournament);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the outputsMerkleRoot parameter of the EpochSealed event:
It is weird to have it as bytes32(0) (invalid).
If there is any valid initial value, it is the pristine Merkle root.
However, I don't see this new event parameter being used from off-chain.
So, do you think this event parameter is necessary?

Copy link
Collaborator Author

@GCdePaula GCdePaula Apr 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this parameter is super necessary on settle.

Here in instantiation, however, not so much. I think we could either:

  • create a new event type without the outputsMerkleRoot;
  • receive a Merkle proof that extracts whatever is there. We have to check whether the machine saves the outputsMerkleRoot in the first yield. It could make sense if we want there to be "initial outputs", (but I don't want this to hinge in the decision since we don't know whether it's a requirement);
  • emit a valid Merkle tree, but "empty" (whatever is the sentinel value we're currently using);
  • emit bytes32(0).

I feel the last one has no apparent issues, and it was the simplest to implement.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think?

Copy link
Collaborator

@guidanoli guidanoli Apr 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having bytes32(0) is harmless because it cannot be used to prove anything.
However, logically speaking, when an epoch is sealed, only the epoch boundaries are defined. Meanwhile, the final machine state after the epoch (and, by extension, the outputs Merkle root) is only defined when it is settled.
So, if you want to notify the off-chain node that an epoch was settled, I think adding an EpochSettled epoch makes more sense. And, besides the outputsMerkleRoot parameter, this event would also include the epoch number and the final machine state hash.

}

function canSettle() external view returns (bool isFinished, uint256 epochNumber, Tree.Node winnerCommitment) {
(isFinished, winnerCommitment,) = _tournament.arbitrationResult();
epochNumber = _epochNumber;
}

function settle(uint256 epochNumber) external {
function settle(uint256 epochNumber, bytes32 outputsMerkleRoot, bytes32[] calldata proof) external {
// Check tournament settlement
uint256 actualEpochNumber = _epochNumber;
require(epochNumber == actualEpochNumber, IncorrectEpochNumber(epochNumber, actualEpochNumber));
require(epochNumber == _epochNumber, IncorrectEpochNumber(epochNumber, _epochNumber));

// Check tournament finished
(bool isFinished,, Machine.Hash finalMachineStateHash) = _tournament.arbitrationResult();
require(isFinished, TournamentNotFinishedYet());
_tournament = ITournament(address(0));

// Seal current accumulating epoch
_epochNumber = ++epochNumber;
uint256 inputIndexLowerBound = _inputIndexUpperBound;
_inputIndexLowerBound = inputIndexLowerBound;
uint256 inputIndexUpperBound = _inputBox.getNumberOfInputs(_appContract);
_inputIndexUpperBound = inputIndexUpperBound;
ITournament tournament = _tournamentFactory.instantiate(finalMachineStateHash, this);
_tournament = tournament;
emit EpochSealed(epochNumber, inputIndexLowerBound, inputIndexUpperBound, finalMachineStateHash, tournament);
// Check outputs Merkle root
_validateOutputTree(finalMachineStateHash, outputsMerkleRoot, proof);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should follow the Checks-Effects-Interactions safety pattern:
Validating the Merkle proof (Checks) should happen before sealing the current accumulating epoch (Effects).

Copy link
Collaborator Author

@GCdePaula GCdePaula Apr 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! I've also simplified the logic, but with a very modest increase in gas costs (< 1%). This function is currently costly because it's instantiating another contract. I feel we should optimize this by using a proxy later.

I've also added a _tournament = ITournament(address(0)); before setting it back to a valid address. I don't think it's really needed (the factory should trusted, so there should be no reentrancy risks), but feels right. I don't know, what do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find a < 1% increase in gas cost perfectly acceptable.
As for zeroing the tournament address before instantiation,
I don't think it's necessary for trusted tournament factories,
but it is a welcome security improvement in general.


// Seal current accumulating epoch, save settled output tree
_epochNumber++;
_inputIndexLowerBound = _inputIndexUpperBound;
_inputIndexUpperBound = _inputBox.getNumberOfInputs(_appContract);
_outputsMerkleRoots[outputsMerkleRoot] = true;

// Start new tournament
_tournament = _tournamentFactory.instantiate(finalMachineStateHash, this);

emit EpochSealed(
_epochNumber,
_inputIndexLowerBound,
_inputIndexUpperBound,
finalMachineStateHash,
outputsMerkleRoot,
_tournament
);
}

function getCurrentSealedEpoch()
Expand Down Expand Up @@ -185,4 +225,37 @@ contract DaveConsensus is IDataProvider {
uint256 log2SizeOfDrive = input.getMinLog2SizeOfDrive();
return input.getMerkleRootFromBytes(log2SizeOfDrive);
}

/// @inheritdoc IOutputsMerkleRootValidator
function isOutputsMerkleRootValid(address appContract, bytes32 outputsMerkleRoot)
public
view
override
returns (bool)
{
require(_appContract == appContract, ApplicationMismatch(_appContract, appContract));
return _outputsMerkleRoots[outputsMerkleRoot];
}

/// @inheritdoc ERC165
function supportsInterface(bytes4 interfaceId) public view override(IERC165, ERC165) returns (bool) {
return interfaceId == type(IDataProvider).interfaceId
|| interfaceId == type(IOutputsMerkleRootValidator).interfaceId || super.supportsInterface(interfaceId);
}

function _validateOutputTree(
Machine.Hash finalMachineStateHash,
bytes32 outputsMerkleRoot,
bytes32[] calldata proof
) internal pure {
bytes32 machineStateHash = Machine.Hash.unwrap(finalMachineStateHash);

require(proof.length == Memory.LOG2_MAX_SIZE, InvalidOutputsMerkleRootProofSize(proof.length));
bytes32 allegedStateHash = proof.merkleRootAfterReplacement(
EmulatorConstants.PMA_CMIO_TX_BUFFER_START >> EmulatorConstants.TREE_LOG2_WORD_SIZE,
keccak256(abi.encode(outputsMerkleRoot))
);

require(machineStateHash == allegedStateHash, InvalidOutputsMerkleRootProof(finalMachineStateHash));
}
}
5 changes: 3 additions & 2 deletions cartesi-rollups/contracts/src/DaveConsensusFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@

pragma solidity ^0.8.8;

import {Create2} from "openzeppelin-contracts/utils/Create2.sol";
import {Create2} from "@openzeppelin-contracts-5.2.0/utils/Create2.sol";

import {IInputBox} from "cartesi-rollups-contracts-2.0.0/inputs/IInputBox.sol";

import {DaveConsensus} from "./DaveConsensus.sol";
import {ITournamentFactory} from "prt-contracts/ITournamentFactory.sol";
import {IInputBox} from "rollups-contracts/inputs/IInputBox.sol";
import {Machine} from "prt-contracts/types/Machine.sol";

/// @title Dave Consensus Factory
Expand Down
Loading
Loading