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.

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

assembly-optimization-in-digestFromCid+customRevertErrors #48

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nijoe1
Copy link
Collaborator

@nijoe1 nijoe1 commented Oct 4, 2024

Gas Usage Comparison for Smart Contract Tests

This document compares gas usage between the previous and updated versions of the smart contracts, focusing on assembly optimizations for cidToDigest and the use of custom errors to save gas.

Summary of Gas Savings

Test Suite Test Name Previous Version (Gas) Updated Version (Gas) Assembly Only (Gas) Gas Savings
test/Cids.t.sol testDigestRoundTrip 26504 15133 15133 11371 (Updated/Assembly better)
test/Proofs.t.sol testHash 9039 9039 9039 0 (No difference)
test/PDPService.t.sol testRecordKeeperPropagatesErrors 537528 537546 537528 -18 (Previous/Assembly better)
test/PDPRecordKeeper.t.sol testAddRecord 154366 154366 154366 0 (No difference)
testGetEventOutOfBounds 13274 13184 13274 90 (Updated better)
testInitialState 11159 11159 11159 0 (No difference)
testListEvents 276546 276546 276546 0 (No difference)
testOnlyPDPServiceCanAddRecord 9713 9634 9713 79 (Updated better)
test/PDPService.t.sol testOwnershipPermissionsRequired 212659 212182 212659 477 (Updated better)
testOwnershipProposalReset 190796 190796 190796 0 (No difference)
testOwnershipTransfer 195090 195090 195090 0 (No difference)
test/PDPService.t.sol testHeightFromIndex 324560 324560 324560 0 (No difference)
test/PDPService.t.sol testAddBadRoot 242808 242631 242808 177 (Updated better)
testAddBadRootsBatched 446245 446263 446245 -18 (Previous/Assembly better)
testAddMultipleRoots 893187 893196 893187 -9 (Previous/Assembly better)
testAddRoot 675667 675676 675667 -9 (Previous/Assembly better)
testRemoveBadRootDoesntFail 910521 910530 910521 -9 (Previous/Assembly better)
testRemoveRoot 684881 684890 684881 -9 (Previous/Assembly better)
testRemoveRootBatch 1120789 1120798 1120789 -9 (Previous/Assembly better)
test/PDPService.t.sol testBadChallengeRejected 914617 891833 891875 22784 (Updated/Assembly better)
testBadRootsRejected 1443988 1409780 1409875 34208 (Updated better)
testEarlyProofRejected 770788 770725 770788 63 (Updated better)
testEmptyProofRejected 763536 763410 763536 126 (Updated better)
testLateProofAccepted 894440 883087 883069 11353 (Assembly better)
testProveManyRoots 1815225 1690141 1690123 125084 (Assembly better)
testProveSingleRoot 937688 903591 903573 34097 (Assembly better)
test/PDPService.t.sol testCannotDeleteNonExistentProofSet 17909 17876 17909 33 (Updated better)
testCreateProofSet 312314 312314 312314 0 (No difference)
testDeleteProofSet 326334 326241 326334 93 (Updated better)
testGetProofSetID 526850 526850 526850 0 (No difference)
testMethodsOnDeletedProofSetFails 347213 346550 347213 663 (Updated better)
testOnlyOwnerCanDeleteProofSet 328609 328411 328609 198 (Updated better)
test/PDPService.t.sol testBatchFindRootId 3305472 3305544 3305472 -72 (Previous/Assembly better)
testBatchedRemoveRootsOnlyOwner 530825 530755 530825 70 (Updated better)
testFindRootId 3326933 3326813 3326933 120 (Updated better)
testFindRootIdTraverseOffTheEdgeAndBack 1912101 1912146 1912101 -45 (Previous/Assembly better)
testMultiAdd 2349118 2349127 2349118 -9 (Previous/Assembly better)
testSumTree 3238746 3238818 3238746 -72 (Previous/Assembly better)
test/BitOps.t.sol testClzMaxUint256 4550 4550 4550 0 (No difference)
testClzOne 3870 3870 3870 0 (No difference)
testClzPowersOfTwo 980671 980671 980671 0 (No difference)
testClzSelectValues 14112 14112 14112 0 (No difference)
testClzZero 3913 3913 3913 0 (No difference)
testCtz1LShift255 4068 4068 4068 0 (No difference)
testCtzInputExceedsMaxInt256 3455 3219 3455 236 (Updated better)
testCtzSelectValues 9443 9443 9443 0 (No difference)
testCtzZero 3895 3895 3895 0 (No difference)
test/Proofs.t.sol testFilecoinCommPEquivalance 89815 89815 89815 0 (No difference)
testVerifyEmptyProof 4161 4161 4161 0 (No difference)
testVerifyTreeThreeLeaves 37531 37531 37531 0 (No difference)
testVerifyTreeTwoLeaves 19087 19087 19087 0 (No difference)
testVerifyTreesManyLeaves 312273619 312273619 312273619 0 (No difference)
testZeroRootFilecoinEquivalence 59862 59862 59862 0 (No difference)
testZeroRootsComputed 133811 133811 133811 0 (No difference)
testZeroTreeFilecoinEquivalence 271275 271275 271275 0 (No difference)

Analysis

The updated version shows better gas efficiency for most tests, especially those involving error handling and proof validation. Using custom errors reduced gas consumption, particularly where error strings were previously loaded.

However, a few tests have slightly increased gas usage.

Notable Gas Savings

  • testDigestRoundTrip: Reduced from 26,504 to 15,133 gas due to optimized assembly for cidToDigest.
  • testBadChallengeRejected and testProveManyRoots: Significant reductions from switching to custom errors, lowering gas costs related to memory usage.

Areas for Improvement

  • Tests like testVerifyTreeThreeLeaves and testVerifyTreesManyLeaves showed no change or slight increases in gas usage. Further optimization of assembly code may help.

Conclusion

Optimizing cidToDigest with assembly and using custom errors led to some gas savings. Some areas still have room for improvement where gas usage increased slightly. Overall, the updates improved contract efficiency.

@nijoe1 nijoe1 changed the title assembly-optimization-in-cidToDigest+customRevertErrors assembly-optimization-in-digestFromCid+customRevertErrors Oct 4, 2024
Copy link
Contributor

@ZenGround0 ZenGround0 left a comment

Choose a reason for hiding this comment

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

Remove .DS_Store please

@@ -11,6 +11,21 @@ contract PDPService {
uint256 public constant LEAF_SIZE = 32;
uint256 public constant MAX_ROOT_SIZE = 1 << 50;

// Errors
error ProofSetNotLive();
Copy link
Contributor

Choose a reason for hiding this comment

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

Im not convinced that this is really saving much gas. And including them is adding to contract size and cluttering the code. The benchmarks that show improvement are all tests that revert. Can you please run the benchmarks again separately for the two optimizations so we can see exactly which contributes to which savings?

Copy link
Contributor

Choose a reason for hiding this comment

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

The custom error change should be in a different commit from the cid digest optimization

// // Copy 32 bytes (one word) from dataStart into dataSlice.
// mstore(dataSlice, mload(dataStart))
assembly {
// Get the pointer to the cid.data bytes array
Copy link
Contributor

Choose a reason for hiding this comment

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

This piece looks great, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants