Skip to content
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

Jump to bottom

feat[verifyMixedAggregatedProof]: duplicateSubmissionIndices as bytes #91

Merged
merged 1 commit into from
Nov 22, 2024
Merged

feat[verifyMixedAggregatedProof]: duplicateSubmissionIndices as bytes #91

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 12 additions & 10 deletions upa/contracts/UpaVerifier.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,8 @@ contract UpaVerifier is

/// Verify a mixed aggregated proof with off-chain and on-chain proofs.
///
/// `duplicateSubmissionIndices` - bytes as packed uint8s
/// indicating the index of the duplicated submission.
/// `proof` - An aggregated proof for the validity of this batch.
/// `proofIds` - The proofIds belonging to this batch. These are assumed
/// to be arranged in the order: [Off-chain, On-chain, Dummy]. Furthermore,
Expand All @@ -636,15 +638,13 @@ contract UpaVerifier is
/// proofIds belongs to an on-chain submission
/// `offChainSubmissionMarkers` - encodes a bool[256] where a `1` marks
/// each proofId that is at the end of an off-chain submission.
/// `duplicateSubmissionIndices` - packed uint8s, reading the lowest-order
/// byte first, indicating the index of the duplicated submission.
function verifyMixedAggregatedProof(
bytes memory duplicateSubmissionIndices,
bytes calldata proof,
bytes32[] calldata proofIds,
uint16 numOffChainProofs,
SubmissionProof[] calldata submissionProofs,
uint256 offChainSubmissionMarkers,
uint256 duplicateSubmissionIndices
uint256 offChainSubmissionMarkers
) external onlyWorker {
// Expected to fit in a uint16 to match the proof counts.
require(proofIds.length <= type(uint16).max, TooManyProofIds());
Expand Down Expand Up @@ -721,14 +721,16 @@ contract UpaVerifier is
// SubmissionProof.
bytes32 submissionId = UpaLib.computeSubmissionId(proofId);

// Interpret `duplicateSubmissionIndices` as packed uint8s,
// reading the lowest-order byte first (shifting below).
uint8 dupSubmissionIdx = uint8(duplicateSubmissionIndices);
// Interpret `duplicateSubmissionIndices` bytes as uint8s
uint256 dupSubmissionSlot;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we explicitly initialize to 0?


(
uint64 submissionIdx,
uint64 submissionBlockNumber
) = getSubmissionIdxAndHeight(submissionId, dupSubmissionIdx);
) = getSubmissionIdxAndHeight(
submissionId,
uint8(duplicateSubmissionIndices[dupSubmissionSlot])
);

if (submissionIdx != 0) {
nextSubmissionIdx = handleSingleProofOnChainSubmission(
Expand Down Expand Up @@ -761,13 +763,13 @@ contract UpaVerifier is
nextSubmissionIdx,
uint16(proofIds.length - 1),
proofIdIdx,
dupSubmissionIdx
uint8(duplicateSubmissionIndices[dupSubmissionSlot])
);

proofIdIdx += proofsThisSubmission;
}

duplicateSubmissionIndices = duplicateSubmissionIndices >> 8;
dupSubmissionSlot++;
}

verifierStorage.nextSubmissionIdxToVerify = nextSubmissionIdx;
Expand Down
3 changes: 1 addition & 2 deletions upa/src/tool/devAggregator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {
} from "../sdk/submissionIntervals";
import {
computeAggregatedProofParameters,
packDupSubmissionIdxs,
packOffChainSubmissionMarkers,
} from "../sdk/aggregatedProofParams";

Expand Down Expand Up @@ -215,12 +214,12 @@ async function submitBatch(

// Submit aggregated proof
await upaInstance.verifier.verifyMixedAggregatedProof(
Uint8Array.from(aggProofParams.dupSubmissionIdxs),
calldata,
proofIds,
aggProofParams.numOffChainProofs,
aggProofParams.submissionProofs,
packOffChainSubmissionMarkers(aggProofParams.offChainSubmissionMarkers),
packDupSubmissionIdxs(aggProofParams.dupSubmissionIdxs),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we remove this function from the SDK? (I may have missed the diff removing it)

options || {}
);

Expand Down
14 changes: 7 additions & 7 deletions upa/test/eventsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
getCallDataForVerifyMixedAggregatedProofTx,
} from "../src/sdk/events";
import { submitProofs } from "../src/sdk/upa";
import { Submission } from "../src/sdk/submission";
import { Submission } from "../src/sdk";
import {
packDupSubmissionIdxs,
packOffChainSubmissionMarkers,
Expand Down Expand Up @@ -300,12 +300,12 @@ describe("EventGetter for events", () => {
const agg1Tx = await verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(proofIds),
proofIds,
proofIds.length - 1,
[sub_1.computeSubmissionProof(0, 1)!.solidity()],
submissionMarkers,
packDupSubmissionIdxs([0])
submissionMarkers
);
return agg1Tx.hash;
})();
Expand All @@ -315,15 +315,15 @@ describe("EventGetter for events", () => {
const agg2Tx = await verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0, 0, 0]),
dummyProofData(proofIds),
proofIds,
0,
[
sub_1.computeSubmissionProof(1, 1)!.solidity(),
sub_3.computeSubmissionProof(0, 1)!.solidity(),
],
packOffChainSubmissionMarkers([]),
packDupSubmissionIdxs([0, 0, 0])
packOffChainSubmissionMarkers([])
);
return agg2Tx.hash;
})();
Expand All @@ -333,12 +333,12 @@ describe("EventGetter for events", () => {
const agg3Tx = await verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(proofIds),
proofIds,
0,
[sub_3.computeSubmissionProof(1, 2)!.solidity()],
packOffChainSubmissionMarkers([]),
packDupSubmissionIdxs([0])
packOffChainSubmissionMarkers([])
);
return agg3Tx.hash;
})();
Expand Down
70 changes: 33 additions & 37 deletions upa/test/offChainSubmissionTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,7 @@ async function checkProofsAndSubmissionVerified(
submission.inputs
);

if (!onChainIsSubmissionVerified) {
return false;
}

return true;
return onChainIsSubmissionVerified;
}

describe("Submissions verified in one aggregation", async () => {
Expand Down Expand Up @@ -287,12 +283,12 @@ describe("Submissions verified in one aggregation", async () => {
const verifyAggProofTx = await upa.verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(proofIds),
proofIds,
numProofsInSubmission,
onChainSubmissionProofs,
offChainSubmissionMarkers,
packDupSubmissionIdxs([0])
offChainSubmissionMarkers
);

await verifyAggProofTx.wait();
Expand Down Expand Up @@ -458,12 +454,12 @@ describe("Submissions verified in one aggregation", async () => {
const verifyAggProofTx = await verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(proofIds),
proofIds,
numProofsInSubmission,
onChainSubmissionProofs,
offChainSubmissionMarkers,
packDupSubmissionIdxs([0])
offChainSubmissionMarkers
);

await verifyAggProofTx.wait();
Expand Down Expand Up @@ -636,12 +632,12 @@ describe("Submissions verified in one aggregation", async () => {
const verifyAggProofTx = await verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(proofIds),
proofIds,
numProofsInOffChainSubmission,
onChainSubmissionProofs,
offChainSubmissionMarkers,
packDupSubmissionIdxs([0])
offChainSubmissionMarkers
);

await verifyAggProofTx.wait();
Expand Down Expand Up @@ -732,12 +728,12 @@ describe("Submissions verified in one aggregation", async () => {
const verifyAggProofTx = await verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(offChainSubmission.proofIds),
offChainSubmission.proofIds,
numProofsInSubmission,
[] /*onChainSubmissionProofs*/,
offChainSubmissionMarkers,
packDupSubmissionIdxs([0])
offChainSubmissionMarkers
);

await verifyAggProofTx.wait();
Expand Down Expand Up @@ -981,12 +977,12 @@ describe("Submissions verified over multiple aggregations", async () => {
const verifyAggProofTx_1 = await verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(proofIds_1),
proofIds_1,
3,
onChainSubmissionProofs_1,
offChainSubmissionMarkers_1,
packDupSubmissionIdxs([0])
offChainSubmissionMarkers_1
);

await verifyAggProofTx_1.wait();
Expand All @@ -1004,12 +1000,12 @@ describe("Submissions verified over multiple aggregations", async () => {
const verifyAggProofTx_2 = await verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(proofIds_2),
proofIds_2,
3,
onChainSubmissionProofs_2,
offChainSubmissionMarkers_2,
packDupSubmissionIdxs([0])
offChainSubmissionMarkers_2
);

await verifyAggProofTx_2.wait();
Expand Down Expand Up @@ -1038,12 +1034,12 @@ describe("Submissions verified over multiple aggregations", async () => {
const verifyAggProofTx_3 = await verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(proofIds_3),
proofIds_3,
3,
onChainSubmissionProofs_3,
offChainSubmissionMarkers_3,
packDupSubmissionIdxs([0])
offChainSubmissionMarkers_3
);

await verifyAggProofTx_3.wait();
Expand Down Expand Up @@ -1303,12 +1299,12 @@ describe("Submissions verified over multiple aggregations", async () => {
const verifyAggProofTx_1 = await verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(proofIds_1),
proofIds_1,
9,
onChainSubmissionProofs_1,
offChainSubmissionMarkers_1,
packDupSubmissionIdxs([0])
offChainSubmissionMarkers_1
);

await verifyAggProofTx_1.wait();
Expand All @@ -1326,12 +1322,12 @@ describe("Submissions verified over multiple aggregations", async () => {
const verifyAggProofTx_2 = await verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(proofIds_2),
proofIds_2,
11,
onChainSubmissionProofs_2,
offChainSubmissionMarkers_2,
packDupSubmissionIdxs([0])
offChainSubmissionMarkers_2
);

await verifyAggProofTx_2.wait();
Expand Down Expand Up @@ -1361,12 +1357,12 @@ describe("Submissions verified over multiple aggregations", async () => {
const verifyAggProofTx_3 = await verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(proofIds_3),
proofIds_3,
0,
onChainSubmissionProofs_3,
offChainSubmissionMarkers_3,
packDupSubmissionIdxs([0])
offChainSubmissionMarkers_3
);

await verifyAggProofTx_3.wait();
Expand Down Expand Up @@ -1765,12 +1761,12 @@ describe("Aggregations containing multiple submissions", async () => {
const verifyAggProofTx_1 = await verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0, 0]),
dummyProofData(firstAggProofIds),
firstAggProofIds,
6,
firstAggSubmissionProofs,
firstAggMarkers,
packDupSubmissionIdxs([0, 0])
firstAggMarkers
);

await verifyAggProofTx_1.wait();
Expand Down Expand Up @@ -1820,12 +1816,12 @@ describe("Aggregations containing multiple submissions", async () => {
const verifyAggProofTx_2 = await verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0, 0]),
dummyProofData(secondAggProofIds),
secondAggProofIds,
9,
secondAggSubmissionProofs,
secondAggMarkers,
packDupSubmissionIdxs([0, 0])
secondAggMarkers
);

await verifyAggProofTx_2.wait();
Expand Down Expand Up @@ -1948,12 +1944,12 @@ describe("Offchain Benchmarks", async () => {
const txResponse = await verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(submission.proofIds),
submission.proofIds,
submissionSize,
[],
packedSubmissionMarkers,
packDupSubmissionIdxs([0])
packedSubmissionMarkers
);

const txReceipt = await txResponse.wait();
Expand Down Expand Up @@ -2120,12 +2116,12 @@ describe("Failure cases", async () => {
verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(proofIds),
proofIds,
numProofsInOffChainSubmission,
onChainSubmissionProofs,
offChainSubmissionMarkers,
packDupSubmissionIdxs([0])
offChainSubmissionMarkers
)
).to.be.revertedWithCustomError(upa.verifier, "InvalidMerkleIntervalProof");
});
Expand Down Expand Up @@ -2369,25 +2365,25 @@ describe("Failure cases", async () => {
verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(proofIds_1),
proofIds_1,
9,
onChainSubmissionProofs_1,
offChainSubmissionMarkers,
packDupSubmissionIdxs([0])
offChainSubmissionMarkers
)
).to.be.revertedWithCustomError(verifier, "InvalidMerkleIntervalProof");

expect(
verifier
.connect(worker)
.verifyMixedAggregatedProof(
Uint8Array.from([0]),
dummyProofData(proofIds_2),
proofIds_2,
9,
onChainSubmissionProofs_2,
offChainSubmissionMarkers,
packDupSubmissionIdxs([0])
offChainSubmissionMarkers
)
).to.be.revertedWithCustomError(verifier, "InvalidMerkleIntervalProof");
});
Expand Down
Loading