feat: Create proof of liveness smart contract #190
Conversation
📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes introduce a new smart contract, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ProofOfLiveness
participant Blockchain
User->>ProofOfLiveness: Call proveLiveness()
ProofOfLiveness->>ProofOfLiveness: Check last proof timestamp
alt If 24 hours have passed
ProofOfLiveness->>Blockchain: Update timestamp
ProofOfLiveness->>User: Emit LivenessProved event
else If not 24 hours
ProofOfLiveness-->>User: Revert with ProofWithinLast24Hours error
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol
Fixed
Show fixed
Hide fixed
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol
Dismissed
Show dismissed
Hide dismissed
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol
Fixed
Show fixed
Hide fixed
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol
Fixed
Show fixed
Hide fixed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (2)
packages/zevm-app-contracts/scripts/proof-of-liveness/deploy.ts (1)
10-23: deployProofOfLiveness function is well-structured, but error handling can be improved.The function correctly deploys the ProofOfLiveness contract, saves its address, and verifies it. However, consider enhancing error handling for deployment and verification steps.
Consider wrapping the deployment and verification steps in try-catch blocks:
const deployProofOfLiveness = async () => { if (!isProtocolNetworkName(networkName)) throw new Error("Invalid network name"); try { const ProofOfLivenessFactory = (await ethers.getContractFactory("ProofOfLiveness")) as ProofOfLiveness__factory; const ProofOfLiveness = await ProofOfLivenessFactory.deploy(); await ProofOfLiveness.deployed(); console.log("ProofOfLiveness deployed to:", ProofOfLiveness.address); saveAddress("ProofOfLiveness", ProofOfLiveness.address, networkName); try { await verifyContract(ProofOfLiveness.address, []); } catch (verifyError) { console.warn("Contract verification failed:", verifyError.message); } } catch (deployError) { console.error("Deployment failed:", deployError.message); throw deployError; } };This structure allows for more granular error handling and ensures that deployment errors are properly caught and re-thrown.
packages/zevm-app-contracts/data/addresses.json (1)
11-12: Approve changes with minor suggestion for consistency.The addition of the "ProofOfLiveness" contract address is consistent with the PR objectives and maintains the existing file structure. However, for improved consistency and future-proofing, consider alphabetizing the contract entries within each network object.
Consider applying the following change for improved consistency:
{ "zevm": { "zeta_testnet": { "disperse": "0x23ce409Ea60c3d75827d04D9db3d52F3af62e44d", - "rewardDistributorFactory": "0xB9dc665610CF5109cE23aBBdaAc315B41FA094c1", - "zetaSwap": "0xA8168Dc495Ed61E70f5c1941e2860050AB902cEF", - "zetaSwapBtcInbound": "0x358E2cfC0E16444Ba7D3164Bbeeb6bEA7472c559", "invitationManager": "0x3649C03C472B698213926543456E9c21081e529d", - "withdrawERC20": "0xa349B9367cc54b47CAb8D09A95836AE8b4D1d84E", - "ZetaXP": "0x5c25b6f4D2b7a550a80561d3Bf274C953aC8be7d", "InstantRewards": "0x10DfEd4ba9b8F6a1c998E829FfC0325D533c80E3", - "ProofOfLiveness": "0x2aD64D83827D102FCBBdEb92DEA91fCAB168Cdc2" + "ProofOfLiveness": "0x2aD64D83827D102FCBBdEb92DEA91fCAB168Cdc2", + "rewardDistributorFactory": "0xB9dc665610CF5109cE23aBBdaAc315B41FA094c1", + "withdrawERC20": "0xa349B9367cc54b47CAb8D09A95836AE8b4D1d84E", + "ZetaXP": "0x5c25b6f4D2b7a550a80561d3Bf274C953aC8be7d", + "zetaSwap": "0xA8168Dc495Ed61E70f5c1941e2860050AB902cEF", + "zetaSwapBtcInbound": "0x358E2cfC0E16444Ba7D3164Bbeeb6bEA7472c559" }, "zeta_mainnet": { // ... (unchanged) } } }This change will enhance readability and make future additions more straightforward.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol (1 hunks)
- packages/zevm-app-contracts/data/addresses.json (1 hunks)
- packages/zevm-app-contracts/scripts/proof-of-liveness/deploy.ts (1 hunks)
🔇 Additional comments not posted (3)
packages/zevm-app-contracts/scripts/proof-of-liveness/deploy.ts (1)
1-8: Imports and initial setup are appropriate.The necessary modules and types are imported, and the network name is correctly extracted from the Hardhat environment. This setup provides a solid foundation for the deployment script.
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol (2)
1-6: Contract declaration and state variable are well-defined.The contract name accurately reflects its purpose, and the state variable
lastProofTimestampis correctly defined as a public mapping to store the last proof timestamp for each user.
8-12: Custom error and event declarations are appropriate.The custom error
ProofWithinLast24Hoursprovides a clear message and includes relevant data. TheLivenessProvedevent is properly defined with an indexed parameter for efficient filtering.
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol
Outdated
Show resolved
Hide resolved
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol
Outdated
Show resolved
Hide resolved
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol
Outdated
Show resolved
Hide resolved
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol
Outdated
Show resolved
Hide resolved
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol
Fixed
Show fixed
Hide fixed
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol
Fixed
Show fixed
Hide fixed
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol (1 hunks)
🧰 Additional context used
GitHub Check: Slither
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol
[notice] 18-32: Block timestamp
ProofOfLiveness.proveLiveness() (contracts/proof-of-liveness/ProofOfLiveness.sol#18-32) uses timestamp for comparisons
Dangerous comparisons:
- currentTime < lastProofTime + PROOF_PERIOD (contracts/proof-of-liveness/ProofOfLiveness.sol#23)
[notice] 35-38: Block timestamp
ProofOfLiveness.canProveLiveness(address) (contracts/proof-of-liveness/ProofOfLiveness.sol#35-38) uses timestamp for comparisons
Dangerous comparisons:
- currentTime >= lastProofTimestamp[user] + PROOF_PERIOD (contracts/proof-of-liveness/ProofOfLiveness.sol#37)
[notice] 41-62: Block timestamp
ProofOfLiveness.getLastPeriodsStatus(address) (contracts/proof-of-liveness/ProofOfLiveness.sol#41-62) uses timestamp for comparisons
Dangerous comparisons:
- lastProofTime >= periodEnd && lastProofTime < periodStart (contracts/proof-of-liveness/ProofOfLiveness.sol#54)
[warning] 45-45: Uninitialized local variables
ProofOfLiveness.getLastPeriodsStatus(address).proofStatus (contracts/proof-of-liveness/ProofOfLiveness.sol#45) is a local variable never initialized
🔇 Additional comments not posted (6)
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol (6)
5-6: Constants are appropriately defined.The use of
PROOF_PERIODandLAST_PERIODS_LENGTHas constants enhances code readability and maintainability.
9-9: MappinglastProofTimestampis correctly declared.The
mapping(address => uint256) public lastProofTimestamp;efficiently tracks the last proof timestamps for each user.
12-12: Custom errorProofWithinLast24Hoursis well-defined.Defining a custom error reduces gas costs when reverting transactions and provides clarity on the revert reason.
15-15: EventLivenessProvedis properly declared.Emitting
LivenessProvedevents enables off-chain services to monitor and record liveness proofs effectively.
18-32:proveLivenessfunction is correctly implemented.The function appropriately restricts users from proving liveness more than once within the
PROOF_PERIOD, updates the timestamp, and emits the corresponding event.🧰 Tools
GitHub Check: Slither
[notice] 18-32: Block timestamp
ProofOfLiveness.proveLiveness() (contracts/proof-of-liveness/ProofOfLiveness.sol#18-32) uses timestamp for comparisons
Dangerous comparisons:
- currentTime < lastProofTime + PROOF_PERIOD (contracts/proof-of-liveness/ProofOfLiveness.sol#23)
35-38:canProveLivenessfunction accurately determines eligibility.The function provides an efficient way for users to check their ability to prove liveness without modifying state.
🧰 Tools
GitHub Check: Slither
[notice] 35-38: Block timestamp
ProofOfLiveness.canProveLiveness(address) (contracts/proof-of-liveness/ProofOfLiveness.sol#35-38) uses timestamp for comparisons
Dangerous comparisons:
- currentTime >= lastProofTimestamp[user] + PROOF_PERIOD (contracts/proof-of-liveness/ProofOfLiveness.sol#37)
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol
Outdated
Show resolved
Hide resolved
andresaiello
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
packages/zevm-app-contracts/test/proof-of-liveness/proof-of-liveness.ts (4)
1-9: Remove unused constant HARDHAT_CHAIN_IDThe constant
HARDHAT_CHAIN_IDis defined but not utilized within the visible code. To maintain code cleanliness and prevent potential confusion, it is advisable to remove unused declarations.Consider applying the following change:
-const HARDHAT_CHAIN_ID = 1337;
27-34: Enhance test coverage for "Should proof" test caseThe current test case effectively verifies the event emission and its parameters. To further strengthen the test, consider adding an assertion to check if the liveness status has been updated correctly in the contract state.
Consider adding the following assertion after line 33:
expect(await proofOfLiveness.getLastPeriodsStatus(owner.address)).to.deep.equal([true, false, false, false, false]);This addition will ensure that the contract's state is updated as expected after proving liveness.
36-78: Well-structured time-based tests with room for minor improvementThe test cases effectively verify the contract's behavior over multiple time periods, including scenarios with consistent proofs and a missed proof. The use of
evm_increaseTimeis appropriate for simulating time passage.To enhance readability and maintainability, consider extracting the time manipulation logic into a separate helper function.
Consider adding a helper function for time manipulation:
async function advanceTime(seconds: number) { await ethers.provider.send("evm_increaseTime", [seconds]); await ethers.provider.send("evm_mine", []); }Then, replace the time advancement code in both test cases with:
await advanceTime(PROOF_PERIOD);This refactoring will reduce code duplication and improve the tests' readability.
91-99: Effective revert test with room for consistency improvementThis test case successfully verifies that the contract reverts when attempting to prove liveness twice within 24 hours. The use of
expect(...).to.be.revertedWithis appropriate for checking the specific error message.To maintain consistency with the other test cases and improve readability, consider using a descriptive error message in the contract and reflecting it in the test.
Update the error message in the contract to be more descriptive:
require(lastProof + 24 hours <= block.timestamp, "Cannot prove liveness more than once in 24 hours");Then, update the test assertion to match the new error message:
await expect(tx).to.be.revertedWith("Cannot prove liveness more than once in 24 hours");This change will make the error message more informative and consistent with best practices.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol (1 hunks)
- packages/zevm-app-contracts/data/addresses.json (1 hunks)
- packages/zevm-app-contracts/test/proof-of-liveness/proof-of-liveness.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol
- packages/zevm-app-contracts/data/addresses.json
🔇 Additional comments not posted (2)
packages/zevm-app-contracts/test/proof-of-liveness/proof-of-liveness.ts (2)
11-25: Test suite setup is well-structuredThe test suite setup is comprehensive and follows testing best practices. The use of
beforeEachto deploy a new contract instance before each test ensures a clean state, promoting test isolation and reliability.
80-89: Comprehensive edge case coverageThis test case effectively verifies the contract's behavior for a single proof, ensuring that only the most recent period is marked as true. The assertion correctly checks the expected outcome, providing valuable coverage for this edge case.
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol
Dismissed
Show dismissed
Hide dismissed
packages/zevm-app-contracts/contracts/proof-of-liveness/ProofOfLiveness.sol
Dismissed
Show dismissed
Hide dismissed
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
packages/zevm-app-contracts/data/addresses.json (1)
Missing Deployment Scripts and Documentation for ProofOfLiveness
The verification process revealed that there are no deployment scripts found for the ProofOfLiveness contract, and the relevant documentation has not been updated accordingly.
Deployment Scripts:
- No deployment script found for
ProofOfLiveness. Please ensure that a deployment script is created and located withinpackages/zevm-app-contracts/scripts/.Documentation Updates:
- No mentions of
ProofOfLivenessfound inREADME.mdor within thedocs/directory. Please update the documentation to include details about the deployment and usage of the new contract.Addressing these issues is crucial to maintain the integrity of the deployment process and to ensure that all team members are informed about the new contract integrations.
🔗 Analysis chain
Line range hint
1-26: Ensure proper documentation and deployment processesWhile the additions to the addresses.json file are correct, it is crucial to ensure that the deployment process and documentation are updated accordingly.
To verify the completeness of the deployment process and documentation, please run the following script:
Additionally, please ensure that:
- The deployment process for the ProofOfLiveness contract is documented.
- Any necessary updates to other parts of the codebase that interact with this new contract are completed.
- The team is aware of the new contract addresses and their implications on the system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for deployment scripts and documentation updates # Check for deployment scripts if grep -q "ProofOfLiveness" packages/zevm-app-contracts/scripts/deploy*.ts; then echo "Deployment script found for ProofOfLiveness." else echo "Warning: No deployment script found for ProofOfLiveness. Please ensure it exists." fi # Check for documentation updates if grep -q "ProofOfLiveness" README.md docs/*.md; then echo "Documentation mentioning ProofOfLiveness found." else echo "Warning: No documentation updates found for ProofOfLiveness. Please update the relevant documentation." fi echo "Verification completed. Address any warnings if present."Length of output: 756
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/zevm-app-contracts/data/addresses.json (2 hunks)
🔇 Additional comments (1)
packages/zevm-app-contracts/data/addresses.json (1)
22-23: New contract address added for ProofOfLiveness in zeta_mainnetThe addition of the ProofOfLiveness contract address for the zeta_mainnet environment is noted. The address format appears valid, and the entry follows the established naming convention.
To ensure the consistency and validity of this addition, please run the following verification script:
#!/bin/bash # Verify the ProofOfLiveness contract address in zeta_mainnet # Check if the address exists in the file if grep -q '"ProofOfLiveness": "0x327c9837B183e69C522a30E6f91A42c86e057432"' packages/zevm-app-contracts/data/addresses.json; then echo "ProofOfLiveness address found in zeta_mainnet section." else echo "Error: ProofOfLiveness address not found or incorrect in zeta_mainnet section." exit 1 fi # Verify the address format if [[ "0x327c9837B183e69C522a30E6f91A42c86e057432" =~ ^0x[a-fA-F0-9]{40}$ ]]; then echo "Address format is valid." else echo "Error: Invalid address format for ProofOfLiveness in zeta_mainnet." exit 1 fi echo "Verification completed successfully."
andresaiello
deleted the
andy/pd-6660-create-proof-of-liveness-smart-contract
branch
Summary
Summary by CodeRabbit
New Features
ProofOfLiveness, allowing users to prove their liveness once every 24 hours.Data Updates
ProofOfLivenesscontract address to the addresses data file for both mainnet and testnet.Deployment
ProofOfLivenesscontract, facilitating its deployment and verification.Testing
ProofOfLivenesscontract, covering various scenarios and edge cases.