From 04b81fba9273e45137c56e9d49805f2bddbdbd6d Mon Sep 17 00:00:00 2001 From: Pouya Date: Sat, 16 Mar 2024 15:49:12 +0100 Subject: [PATCH 1/2] Fix set params issues --- contracts/Staking.sol | 88 +++++++++++++++++++++++++++++--- docs/UnchainedStaking.md | 53 +++++++------------- docs/console.md | 12 +++++ test/Staking.test.js | 105 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 215 insertions(+), 43 deletions(-) create mode 100644 docs/console.md diff --git a/contracts/Staking.sol b/contracts/Staking.sol index f6f068d..7b79839 100644 --- a/contracts/Staking.sol +++ b/contracts/Staking.sol @@ -9,6 +9,8 @@ import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; +import "hardhat/console.sol"; + // TODO: Should there be a max voting power? /** @@ -109,6 +111,15 @@ contract UnchainedStaking is Ownable, IERC721Receiver, ReentrancyGuard { uint256 nonce; } + struct EIP712SetParamsKey { + address token; + address nft; + uint256 threshold; + uint256 expiration; + address collector; + uint256 nonce; + } + struct EIP712SetSigner { address staker; address signer; @@ -130,12 +141,10 @@ contract UnchainedStaking is Ownable, IERC721Receiver, ReentrancyGuard { error NotConsumer(uint256 index); error InvalidSignature(uint256 index); error AlreadyAccused(uint256 index); - error AlreadySlashed(uint256 index); error VotingPowerZero(uint256 index); error AlreadyVoted(uint256 index); - error AlreadyAccepted(uint256 index); error TopicExpired(uint256 index); - error StakeExpiredBeforeVote(uint256 index); + error StakeExpiresBeforeVote(uint256 index); mapping(address => mapping(uint256 => bool)) private _nonces; @@ -169,6 +178,11 @@ contract UnchainedStaking is Ownable, IERC721Receiver, ReentrancyGuard { "EIP712SetParams(address requester,address token,address nft,uint256 threshold,uint256 expiration,address collector,uint256 nonce)" ); + bytes32 constant EIP712_SET_PARAMS_KEY_TYPEHASH = + keccak256( + "EIP712SetParams(address token,address nft,uint256 threshold,uint256 expiration,address collector,uint256 nonce)" + ); + uint256 private _consensusLock; uint256 private _consensusThreshold = 51; uint256 private _votingTopicExpiration = 1 days; @@ -437,6 +451,35 @@ contract UnchainedStaking is Ownable, IERC721Receiver, ReentrancyGuard { ); } + /** + * @dev Computes the EIP-712 compliant hash of a set of parameters intended for a specific operation. + * This operation could involve setting new contract parameters such as token address, NFT address, + * a threshold value, a collector address, and a nonce for operation uniqueness. The hash is created + * following the EIP-712 standard, which allows for securely signed data to be verified by the contract. + * This function is internal and pure, meaning it doesn't alter or read the contract's state. + * @param eip712SetParamsKey The struct containing the parameters to be hashed. This includes token and + * NFT addresses, a threshold value for certain operations, a collector address that may receive funds + * or penalties, and a nonce to ensure the hash's uniqueness. + * @return The EIP-712 compliant hash of the provided parameters, which can be used to verify signatures + * or as a key in mappings. + */ + function hash( + EIP712SetParamsKey memory eip712SetParamsKey + ) internal pure returns (bytes32) { + return + keccak256( + abi.encode( + EIP712_SET_PARAMS_KEY_TYPEHASH, + eip712SetParamsKey.token, + eip712SetParamsKey.nft, + eip712SetParamsKey.threshold, + eip712SetParamsKey.expiration, + eip712SetParamsKey.collector, + eip712SetParamsKey.nonce + ) + ); + } + /** * @dev Called by a user to stake their tokens along with NFTs if desired, specifying whether the stake is for a consumer. * @param duration The duration for which the tokens and NFTs are staked. @@ -851,7 +894,7 @@ contract UnchainedStaking is Ownable, IERC721Receiver, ReentrancyGuard { EIP712Slash memory eip712Slash = eip712Slashes[i]; if (_incidentTracker[eip712Slash.incident]) { - revert AlreadySlashed(i); + continue; } EIP712SlashKey memory slashKey = EIP712SlashKey( @@ -879,7 +922,7 @@ contract UnchainedStaking is Ownable, IERC721Receiver, ReentrancyGuard { } if (userStake.unlock <= expires) { - revert StakeExpiredBeforeVote(i); + revert StakeExpiresBeforeVote(i); } Slash storage slashData = _slashes[eipHash]; @@ -1007,10 +1050,19 @@ contract UnchainedStaking is Ownable, IERC721Receiver, ReentrancyGuard { EIP712SetParams memory eip712SetParam = eip712SetParams[i]; if (_setParamsTracker[eip712SetParam.nonce]) { - revert AlreadyAccepted(i); + continue; } - bytes32 eipHash = hash(eip712SetParam); + EIP712SetParamsKey memory key = EIP712SetParamsKey( + eip712SetParam.token, + eip712SetParam.nft, + eip712SetParam.threshold, + eip712SetParam.expiration, + eip712SetParam.collector, + eip712SetParam.nonce + ); + + bytes32 eipHash = hash(key); if (_firstReported[eipHash] == 0) { _firstReported[eipHash] = block.timestamp; @@ -1029,7 +1081,7 @@ contract UnchainedStaking is Ownable, IERC721Receiver, ReentrancyGuard { } if (userStake.unlock <= expires) { - revert StakeExpiredBeforeVote(i); + revert StakeExpiresBeforeVote(i); } Params storage setParamsData = _setParams[eipHash]; @@ -1094,6 +1146,26 @@ contract UnchainedStaking is Ownable, IERC721Receiver, ReentrancyGuard { return setParamsData.info; } + /** + * @dev Retrieves the current contract parameters, including the token and NFT addresses, + * the consensus threshold, the voting topic expiration, and the collector address for + * slash penalties. This function returns the current state of the contract's parameters. + * @return ParamsInfo A struct containing the current contract parameters. + */ + function getParams() external view returns (ParamsInfo memory) { + return + ParamsInfo( + address(_token), + address(_nft), + _consensusThreshold, + _votingTopicExpiration, + _slashCollectionAddr, + 0, + 0, + true + ); + } + /** * @dev Checks if a specific address has already requested a set of parameter updates. This * is useful for verifying participation in the consensus process for a parameter update. diff --git a/docs/UnchainedStaking.md b/docs/UnchainedStaking.md index 867bea8..a4ba394 100644 --- a/docs/UnchainedStaking.md +++ b/docs/UnchainedStaking.md @@ -150,6 +150,23 @@ function getHasSlashed(UnchainedStaking.EIP712SlashKey key, address slasher) ext |---|---|---| | _0 | bool | undefined | +### getParams + +```solidity +function getParams() external view returns (struct UnchainedStaking.ParamsInfo) +``` + + + +*Retrieves the current contract parameters, including the token and NFT addresses, the consensus threshold, the voting topic expiration, and the collector address for slash penalties. This function returns the current state of the contract's parameters.* + + +#### Returns + +| Name | Type | Description | +|---|---|---| +| _0 | UnchainedStaking.ParamsInfo | ParamsInfo A struct containing the current contract parameters. | + ### getSetParamsData ```solidity @@ -852,22 +869,6 @@ error AddressZero() -### AlreadyAccepted - -```solidity -error AlreadyAccepted(uint256 index) -``` - - - - - -#### Parameters - -| Name | Type | Description | -|---|---|---| -| index | uint256 | undefined | - ### AlreadyAccused ```solidity @@ -878,22 +879,6 @@ error AlreadyAccused(uint256 index) -#### Parameters - -| Name | Type | Description | -|---|---|---| -| index | uint256 | undefined | - -### AlreadySlashed - -```solidity -error AlreadySlashed(uint256 index) -``` - - - - - #### Parameters | Name | Type | Description | @@ -1155,10 +1140,10 @@ error SafeERC20FailedOperation(address token) |---|---|---| | token | address | undefined | -### StakeExpiredBeforeVote +### StakeExpiresBeforeVote ```solidity -error StakeExpiredBeforeVote(uint256 index) +error StakeExpiresBeforeVote(uint256 index) ``` diff --git a/docs/console.md b/docs/console.md new file mode 100644 index 0000000..8bab67a --- /dev/null +++ b/docs/console.md @@ -0,0 +1,12 @@ +# console + + + + + + + + + + + diff --git a/test/Staking.test.js b/test/Staking.test.js index ac8efce..2d7a0a4 100644 --- a/test/Staking.test.js +++ b/test/Staking.test.js @@ -4,11 +4,56 @@ const { randomBytes } = require("crypto"); const zipIndex = (arr) => arr.map((item, i) => [item, i]); +const EIP712_TYPES = { + EIP712Domain: [ + { name: "name", type: "string" }, + { name: "version", type: "string" }, + { name: "chainId", type: "uint256" }, + { name: "verifyingContract", type: "address" }, + ], + EIP712Transfer: [ + { name: "from", type: "address" }, + { name: "to", type: "address" }, + { name: "amount", type: "uint256" }, + { name: "nonces", type: "uint256[]" }, + ], + EIP712Slash: [ + { name: "accused", type: "address" }, + { name: "accuser", type: "address" }, + { name: "amount", type: "uint256" }, + { name: "incident", type: "bytes32" }, + ], + EIP712SlashKey: [ + { name: "accused", type: "address" }, + { name: "amount", type: "uint256" }, + { name: "incident", type: "bytes32" }, + ], + EIP712SetParams: [ + { name: "requester", type: "address" }, + { name: "token", type: "address" }, + { name: "nft", type: "address" }, + { name: "threshold", type: "uint256" }, + { name: "expiration", type: "uint256" }, + { name: "collector", type: "address" }, + { name: "nonce", type: "uint256" }, + ], + EIP712SetSigner: [ + { name: "staker", type: "address" }, + { name: "signer", type: "address" }, + ], +}; + +const signEip712 = async (signer, domain, types, message) => { + const signature = await signer.signTypedData(domain, types, message); + return ethers.Signature.from(signature); +}; + describe("Staking", function () { let staking, token, nft; let owner, user1, user2, user3, user4; let stakingAddr, tokenAddr, nftAddr; let user1bls, user2bls, user3bls, user4bls; + let eip712domain; beforeEach(async function () { [owner, user1, user2, user3, user4] = await ethers.getSigners(); @@ -34,12 +79,19 @@ describe("Staking", function () { nftAddr, 10, owner.address, - "UnchainedStaking", + "Unchained", "1" ); stakingAddr = await staking.getAddress(); + eip712domain = { + name: "Unchained", + version: "1", + chainId: await staking.getChainId(), + verifyingContract: stakingAddr, + }; + // Send tokens and NFTs from owner to users for (const [user, userIndex] of zipIndex([user1, user2, user3, user4])) { await token.transfer(user.address, ethers.parseUnits("100000")); @@ -148,4 +200,55 @@ describe("Staking", function () { expect(postIncreaseStake.amount).to.equal(ethers.parseUnits("1000")); expect(postIncreaseStake.nftIds.length).to.equal(2); }); + + it("correctly changes the contract parameters with majority vote", async function () { + // Stake tokens for each user + for (const user of [user1, user2, user3, user4]) { + await token.connect(user).approve(stakingAddr, ethers.parseUnits("500")); + await staking + .connect(user) + .stake(25 * 60 * 60 * 24, ethers.parseUnits("500"), [], false); + } + + // Sign EIP712 message for SetParams + const messages = []; + const signatures = []; + + const params = { + token: tokenAddr, + nft: nftAddr, + threshold: 60, + expiration: 60 * 60 * 24 * 7, + collector: owner.address, + nonce: 0, + }; + + for (const user of [user1, user2, user3]) { + const message = { + requester: user.address, + ...params, + }; + + const signed = await signEip712( + user, + eip712domain, + { EIP712SetParams: EIP712_TYPES.EIP712SetParams }, + message + ); + + messages.push(message); + signatures.push(signed); + } + + // Set the parameters + await staking.connect(owner).setParams(messages, signatures); + + // Check the parameters + const contractParams = await staking.getParams(); + expect(contractParams.token).to.equal(params.token); + expect(contractParams.nft).to.equal(params.nft); + expect(contractParams.threshold).to.equal(params.threshold); + expect(contractParams.expiration).to.equal(params.expiration); + expect(contractParams.collector).to.equal(params.collector); + }); }); From 70ded2da250ce7c3060934b2fc2c4b6e68689e16 Mon Sep 17 00:00:00 2001 From: Pouya Date: Sat, 16 Mar 2024 15:52:49 +0100 Subject: [PATCH 2/2] Remove hardhat console --- contracts/Staking.sol | 2 -- 1 file changed, 2 deletions(-) diff --git a/contracts/Staking.sol b/contracts/Staking.sol index 7b79839..dc0813e 100644 --- a/contracts/Staking.sol +++ b/contracts/Staking.sol @@ -9,8 +9,6 @@ import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; -import "hardhat/console.sol"; - // TODO: Should there be a max voting power? /**