Skip to content

Commit 688b3a1

Browse files
authored
fix!: bug of verifyMultiRowRootsToDataRootTupleRoot (#307)
The original `_root` parameter is not verified before being used in `verifyMultiRowRootsToDataRootTupleRootProof`, it will allow arbitrary `_rowRoots` to be proved valid. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Enhanced data integrity checks by updating the verification process in the application. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 6b5839d commit 688b3a1

File tree

3 files changed

+15
-29
lines changed

3 files changed

+15
-29
lines changed

src/lib/verifier/DAVerifier.sol

+9-12
Original file line numberDiff line numberDiff line change
@@ -78,17 +78,16 @@ library DAVerifier {
7878
/// @notice Verifies that the shares, which were posted to Celestia, were committed to by the Blobstream smart contract.
7979
/// @param _bridge The Blobstream smart contract instance.
8080
/// @param _sharesProof The proof of the shares to the data root tuple root.
81-
/// @param _root The data root of the block that contains the shares.
8281
/// @return `true` if the proof is valid, `false` otherwise.
8382
/// @return an error code if the proof is invalid, ErrorCodes.NoError otherwise.
84-
function verifySharesToDataRootTupleRoot(IDAOracle _bridge, SharesProof memory _sharesProof, bytes32 _root)
83+
function verifySharesToDataRootTupleRoot(IDAOracle _bridge, SharesProof memory _sharesProof)
8584
internal
8685
view
8786
returns (bool, ErrorCodes)
8887
{
8988
// checking that the data root was committed to by the Blobstream smart contract.
9089
(bool success, ErrorCodes errorCode) = verifyMultiRowRootsToDataRootTupleRoot(
91-
_bridge, _sharesProof.rowRoots, _sharesProof.rowProofs, _sharesProof.attestationProof, _root
90+
_bridge, _sharesProof.rowRoots, _sharesProof.rowProofs, _sharesProof.attestationProof
9291
);
9392
if (!success) {
9493
return (false, errorCode);
@@ -100,7 +99,7 @@ library DAVerifier {
10099
_sharesProof.namespace,
101100
_sharesProof.rowRoots,
102101
_sharesProof.rowProofs,
103-
_root
102+
_sharesProof.attestationProof.tuple.dataRoot
104103
);
105104

106105
return (valid, error);
@@ -164,15 +163,13 @@ library DAVerifier {
164163
/// @param _bridge The Blobstream smart contract instance.
165164
/// @param _rowRoot The row/column root to be proven.
166165
/// @param _rowProof The proof of the row/column root to the data root.
167-
/// @param _root The data root of the block that contains the row.
168166
/// @return `true` if the proof is valid, `false` otherwise.
169167
/// @return an error code if the proof is invalid, ErrorCodes.NoError otherwise.
170168
function verifyRowRootToDataRootTupleRoot(
171169
IDAOracle _bridge,
172170
NamespaceNode memory _rowRoot,
173171
BinaryMerkleProof memory _rowProof,
174-
AttestationProof memory _attestationProof,
175-
bytes32 _root
172+
AttestationProof memory _attestationProof
176173
) internal view returns (bool, ErrorCodes) {
177174
// checking that the data root was committed to by the Blobstream smart contract
178175
if (
@@ -183,7 +180,8 @@ library DAVerifier {
183180
return (false, ErrorCodes.InvalidDataRootTupleToDataRootTupleRootProof);
184181
}
185182

186-
(bool valid, ErrorCodes error) = verifyRowRootToDataRootTupleRootProof(_rowRoot, _rowProof, _root);
183+
(bool valid, ErrorCodes error) =
184+
verifyRowRootToDataRootTupleRootProof(_rowRoot, _rowProof, _attestationProof.tuple.dataRoot);
187185

188186
return (valid, error);
189187
}
@@ -213,15 +211,13 @@ library DAVerifier {
213211
/// @param _bridge The Blobstream smart contract instance.
214212
/// @param _rowRoots The set of row/column roots to be proved.
215213
/// @param _rowProofs The set of proofs of the _rowRoots in the same order.
216-
/// @param _root The data root of the block that contains the rows.
217214
/// @return `true` if the proof is valid, `false` otherwise.
218215
/// @return an error code if the proof is invalid, ErrorCodes.NoError otherwise.
219216
function verifyMultiRowRootsToDataRootTupleRoot(
220217
IDAOracle _bridge,
221218
NamespaceNode[] memory _rowRoots,
222219
BinaryMerkleProof[] memory _rowProofs,
223-
AttestationProof memory _attestationProof,
224-
bytes32 _root
220+
AttestationProof memory _attestationProof
225221
) internal view returns (bool, ErrorCodes) {
226222
// checking that the data root was committed to by the Blobstream smart contract
227223
if (
@@ -233,7 +229,8 @@ library DAVerifier {
233229
}
234230

235231
// checking that the rows roots commit to the data root.
236-
(bool valid, ErrorCodes error) = verifyMultiRowRootsToDataRootTupleRootProof(_rowRoots, _rowProofs, _root);
232+
(bool valid, ErrorCodes error) =
233+
verifyMultiRowRootsToDataRootTupleRootProof(_rowRoots, _rowProofs, _attestationProof.tuple.dataRoot);
237234

238235
return (valid, error);
239236
}

src/lib/verifier/test/DAVerifier.t.sol

+4-10
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,7 @@ contract DAVerifierTest is DSTest {
126126
SharesProof memory sharesProof =
127127
SharesProof(_data, _shareProofs, fixture.getNamespace(), _rowRoots, _rowProofs, attestationProof);
128128

129-
(bool valid, DAVerifier.ErrorCodes errorCode) =
130-
DAVerifier.verifySharesToDataRootTupleRoot(bridge, sharesProof, fixture.dataRoot());
129+
(bool valid, DAVerifier.ErrorCodes errorCode) = DAVerifier.verifySharesToDataRootTupleRoot(bridge, sharesProof);
131130
assertTrue(valid);
132131
assertEq(uint8(errorCode), uint8(DAVerifier.ErrorCodes.NoError));
133132
}
@@ -138,11 +137,7 @@ contract DAVerifierTest is DSTest {
138137
);
139138

140139
(bool valid, DAVerifier.ErrorCodes errorCode) = DAVerifier.verifyRowRootToDataRootTupleRoot(
141-
bridge,
142-
fixture.getFirstRowRootNode(),
143-
fixture.getRowRootToDataRootProof(),
144-
attestationProof,
145-
fixture.dataRoot()
140+
bridge, fixture.getFirstRowRootNode(), fixture.getRowRootToDataRootProof(), attestationProof
146141
);
147142
assertTrue(valid);
148143
assertEq(uint8(errorCode), uint8(DAVerifier.ErrorCodes.NoError));
@@ -159,9 +154,8 @@ contract DAVerifierTest is DSTest {
159154
fixture.dataRootTupleRootNonce(), fixture.getDataRootTuple(), fixture.getDataRootTupleProof()
160155
);
161156

162-
(bool valid, DAVerifier.ErrorCodes errorCode) = DAVerifier.verifyMultiRowRootsToDataRootTupleRoot(
163-
bridge, _rowRoots, _rowProofs, attestationProof, fixture.dataRoot()
164-
);
157+
(bool valid, DAVerifier.ErrorCodes errorCode) =
158+
DAVerifier.verifyMultiRowRootsToDataRootTupleRoot(bridge, _rowRoots, _rowProofs, attestationProof);
165159
assertTrue(valid);
166160
assertEq(uint8(errorCode), uint8(DAVerifier.ErrorCodes.NoError));
167161
}

src/lib/verifier/test/RollupInclusionProofs.t.sol

+2-7
Original file line numberDiff line numberDiff line change
@@ -268,11 +268,7 @@ contract RollupInclusionProofTest is DSTest {
268268
);
269269
bool success;
270270
(success, error) = DAVerifier.verifyRowRootToDataRootTupleRoot(
271-
bridge,
272-
fixture.getFirstRowRootNode(),
273-
fixture.getRowRootToDataRootProof(),
274-
attestationProof,
275-
fixture.dataRoot()
271+
bridge, fixture.getFirstRowRootNode(), fixture.getRowRootToDataRootProof(), attestationProof
276272
);
277273
assertTrue(success);
278274
assertEq(uint8(error), uint8(DAVerifier.ErrorCodes.NoError));
@@ -323,8 +319,7 @@ contract RollupInclusionProofTest is DSTest {
323319

324320
// let's authenticate the share proof to the data root tuple root to be sure that
325321
// the square size is valid.
326-
(bool success, DAVerifier.ErrorCodes error) =
327-
DAVerifier.verifySharesToDataRootTupleRoot(bridge, shareProof, fixture.dataRoot());
322+
(bool success, DAVerifier.ErrorCodes error) = DAVerifier.verifySharesToDataRootTupleRoot(bridge, shareProof);
328323
assertTrue(success);
329324
assertEq(uint8(error), uint8(DAVerifier.ErrorCodes.NoError));
330325

0 commit comments

Comments
 (0)