feat: Requirement tests for Staking #3
Conversation
| @@ -38,6 +40,7 @@ contract GameVault is ERC721Wrapper, ObjectRegistryClient, IGameVault { | |||
| id, | |||
| msg.sender, | |||
| block.number - wasStakedAt | |||
| // TODO param expects just `stakedAt[i]` not `block.number - stakedAt[i]` | |||
| @@ -4,7 +4,9 @@ pragma solidity ^0.8.19; | |||
| import {IObjectRegistry} from "./interfaces/IObjectRegistry.sol"; | |||
|
|
|||
| contract ObjectRegistry is IObjectRegistry { | |||
| bytes32 internal constant OWNER = "Owner"; | |||
| bytes32 internal constant OWNER = "Owner"; // maybe put all constants in a library? | |||
There was a problem hiding this comment.
Yeah that may be better. This way does show you exactly which ones are being used by a contract but that can still be accomplished.
| @@ -19,7 +21,7 @@ contract GameVault is ERC721Wrapper, ObjectRegistryClient, IGameVault { | |||
| string memory stakedTokenName, ///name of tokens that this contract issues on stake of underlyingToken | |||
| string memory stakedTokenSymbol, ///symbol of tokens that this contract issues on stake of underlyingToken | |||
| IObjectRegistry registry, | |||
| bytes32 game | |||
| bytes32 game // TODO unused | |||
| @@ -80,6 +81,7 @@ contract Seasons is ObjectRegistryClient, ISeasons { | |||
| IXP(registry.addressOf(XP)).awardXP( | |||
| to, | |||
| STAKER_XP_REWARD * (block.number - stakedAt) | |||
| // TODO are `rewardsPerBlock` and `STAKER_XP_REWARD` expected to be the same? | |||
There was a problem hiding this comment.
no one is vault tokens one is XP
| @@ -58,11 +58,14 @@ contract StakerRewards is ObjectRegistryClient, IStakerRewards { | |||
| numBlocks = block.number - stakedAt; | |||
| } | |||
| claimedAt[id] = block.number; | |||
| // TODO should this include a zero check so we don't call unnecessarily? | |||
There was a problem hiding this comment.
Maybe, but that also unnecessarily checks for 0 every time then, where it should usually not happen that the value is actually 0.
but, checking 0 is a common approach.
| rewardToken.transfer(to, rewardPerBlock * numBlocks); | ||
| } | ||
|
|
||
| function claim(uint id) external override { | ||
| require( | ||
| // TODO This `ownerOf` call will be incorrect | ||
| // The owner of the NFT while it is staked is the game vault, not the user | ||
| underlyingToken.ownerOf(id) == msg.sender, |
There was a problem hiding this comment.
Yes needs fixed, it needs to call the token at the GameVault address, not the underlying token
| const gameNameBytes = hre.ethers.utils.formatBytes32String(gameName); | ||
| await games.createGame(gameNameBytes, deployer.address, "a-staking-game"); | ||
|
|
||
| // Registry for the StakingGame |
There was a problem hiding this comment.
Yes you successfully get the gameObjects here, which is the ObjectRegistry for the game you're in.
There was a problem hiding this comment.
(worth pointing out as this is a complicated part of the system. it is one step removed from just inheriting the ObjectRegistry in Games.sol, the simplest and first way I did it)
| @@ -128,6 +128,8 @@ describe("ZXP", () => { | |||
| await mockErc721.connect(deployer).mint(s1, s1nft); | |||
| }); | |||
| it("Staker 1 stakes NFT", async () => { | |||
| // how does user get staked nft back? is it burned on unstake? | |||
There was a problem hiding this comment.
yes it is burned on withdrawTo in ERC721Wrapper
| @@ -138,6 +140,8 @@ describe("ZXP", () => { | |||
| await mockErc721.connect(staker2)["safeTransferFrom(address,address,uint256)"](s2, gameVault.address, s2nft); | |||
| }); | |||
| it("Gets season registry", async () => { | |||
| // shouldnt I be able to get this info through the gameRegistry? | |||
| // why do we have another registry? | |||
There was a problem hiding this comment.
These are the ObjectRegistrys stored in the Seasons struct.
It is a separate set of contracts stored and managed separately from the gameRegistrys set of contracts.
We're able to make different rules on this set of contracts for how they can be updated and added.
No description provided.