Open peterduhon opened 2 weeks ago
Input validation and incomplete implementation. Let's break down the improvements:
Input Validation:
In the startGame() function:
I added checks to ensure the game isn't already active, there are enough players, and there's enough LINK token balance for the Chainlink VRF request.
In the manageTurn() function:
I added checks to ensure it's the correct player's turn, the player hasn't folded, the bet amount is valid, and the player has sufficient balance.
// Input Validation and Function Implementation Improvements
// 1. Input Validation
// Example: startGame function function startGame() external onlyOwner { require(!gameActive, "PokerGame: Game is already in progress"); require(playerAddresses.length >= 2, "PokerGame: Not enough players to start the game"); require(LINK.balanceOf(address(this)) >= fee, "PokerGame: Not enough LINK to request randomness");
gameActive = true;
currentPot = 0;
currentPlayerIndex = 0;
currentPhase = GamePhase.PreFlop;
communityCardCount = 0;
collectBuyIns();
bytes32 requestId = requestRandomness(keyHash, fee);
pendingRequests[requestId] = true;
emit RandomnessRequested(requestId);
emit GameStarted();
}
// Example: manageTurn function function manageTurn(uint256 betAmount) external isGameActive isPlayer { require(msg.sender == playerAddresses[currentPlayerIndex], "PokerGame: It's not your turn to play"); require(players[msg.sender].action != PlayerAction.Fold, "PokerGame: You have already folded"); require(betAmount >= currentRound.betAmount || betAmount == 0, "PokerGame: Invalid bet amount"); require(players[msg.sender].balance >= betAmount, "PokerGame: Insufficient balance");
if (betAmount == 0) {
fold();
} else if (betAmount == currentRound.betAmount) {
call();
} else if (betAmount > currentRound.betAmount) {
raise(betAmount);
} else {
revert("PokerGame: Invalid bet amount");
}
emit TurnManaged(msg.sender);
nextPlayerTurn();
}
// 2. Incomplete Implementation
// Example: distributePots function function distributePots() internal { require(currentPhase == GamePhase.Showdown, "PokerGame: Cannot distribute pots before showdown");
// Distribute main pot
address[] memory mainWinners = determineWinners();
uint256 mainPotShare = mainPot / mainWinners.length;
for (uint256 i = 0; i < mainWinners.length; i++) {
players[mainWinners[i]].balance += mainPotShare;
emit PayoutProcessed(mainWinners[i], mainPotShare);
}
// Distribute side pots
for (uint256 i = 0; i < sidePots.length; i++) {
address[] memory sideWinners = determineWinnersForSidePot(sidePots[i].eligiblePlayers);
uint256 sidePotShare = sidePots[i].amount / sideWinners.length;
for (uint256 j = 0; j < sideWinners.length; j++) {
players[sideWinners[j]].balance += sidePotShare;
emit PayoutProcessed(sideWinners[j], sidePotShare);
}
}
// Reset game state
resetGameState();
}
// Helper function to reset game state after pot distribution function resetGameState() internal { gameActive = false; mainPot = 0; delete sidePots; currentPhase = GamePhase.PreFlop; communityCardCount = 0; delete communityCards;
for (uint256 i = 0; i < playerAddresses.length; i++) {
delete players[playerAddresses[i]].hand;
players[playerAddresses[i]].action = PlayerAction.Begin;
players[playerAddresses[i]].isAllIn = false;
}
emit GameEnded();
}
// Additional improvements for manageTurn function fold() internal { players[msg.sender].action = PlayerAction.Fold; removePlayerFromActiveList(msg.sender); emit PlayerFolded(msg.sender);
if (getCurrentActivePlayers() == 1) {
endGame(getLastActivePlayer());
}
}
function call() internal { uint256 callAmount = currentRound.betAmount - currentRound.playerBets[msg.sender]; require(players[msg.sender].balance >= callAmount, "PokerGame: Insufficient balance to call");
players[msg.sender].balance -= callAmount;
currentRound.playerBets[msg.sender] += callAmount;
currentRound.totalPot += callAmount;
mainPot += callAmount;
emit PlayerCalled(msg.sender, callAmount);
if (isRoundComplete()) {
advancePhase();
}
}
function raise(uint256 amount) internal { require(amount > currentRound.betAmount, "PokerGame: Raise amount must be higher than current bet"); uint256 raiseAmount = amount - currentRound.playerBets[msg.sender]; require(players[msg.sender].balance >= raiseAmount, "PokerGame: Insufficient balance to raise");
players[msg.sender].balance -= raiseAmount;
currentRound.playerBets[msg.sender] += raiseAmount;
currentRound.totalPot += raiseAmount;
currentRound.betAmount = amount;
mainPot += raiseAmount;
emit PlayerRaised(msg.sender, raiseAmount);
resetPlayersActed();
}
function getCurrentActivePlayers() internal view returns (uint256) { uint256 activePlayers = 0; for (uint256 i = 0; i < playerAddresses.length; i++) { if (players[playerAddresses[i]].action != PlayerAction.Fold) { activePlayers++; } } return activePlayers; }
function getLastActivePlayer() internal view returns (address) { for (uint256 i = 0; i < playerAddresses.length; i++) { if (players[playerAddresses[i]].action != PlayerAction.Fold) { return playerAddresses[i]; } } revert("PokerGame: No active players found"); }
function isRoundComplete() internal view returns (bool) { for (uint256 i = 0; i < playerAddresses.length; i++) { if (players[playerAddresses[i]].action != PlayerAction.Fold && currentRound.playerBets[playerAddresses[i]] < currentRound.betAmount) { return false; } } return true; }
function resetPlayersActed() internal { for (uint256 i = 0; i < playerAddresses.length; i++) { if (players[playerAddresses[i]].action != PlayerAction.Fold) { players[playerAddresses[i]].action = PlayerAction.Begin; } } }
In the GameManagement contract, add:
import "./HandEvaluator.sol";
import "./UserManagement.sol";
contract GameManagement is Ownable, AccessControl {
HandEvaluator public handEvaluator;
UserManagement public userManagement;
constructor(address _handEvaluator, address _userManagement) {
handEvaluator = HandEvaluator(_handEvaluator);
userManagement = UserManagement(_userManagement);
}
function createGameRoom(uint256 _buyInAmount, uint256 _maxPlayers) external onlyRole(ADMIN_ROLE) {
// ... existing code ...
// Initialize game in HandEvaluator
handEvaluator.initializeGame(gameId, _buyInAmount, _maxPlayers);
}
function startGame(uint256 _gameId) external onlyRole(ADMIN_ROLE) {
require(gameRooms[_gameId].status == GameStatus.Waiting, "Game not in waiting state");
handEvaluator.startGame(_gameId);
gameRooms[_gameId].status = GameStatus.Active;
}
}
In the HandEvaluator contract, modify the betting functions to update balances in UserManagement:
import "./UserManagement.sol";
contract HandEvaluator is VRFConsumerBase, Ownable, ReentrancyGuard {
UserManagement public userManagement;
constructor(address _userManagement, /* other parameters */) {
userManagement = UserManagement(_userManagement);
}
function placeBet(uint256 amount) public isGameActive isPlayer {
require(amount >= currentRound.betAmount, "PokerGame: Bet amount too low");
require(userManagement.getUserBalance(msg.sender) >= amount, "PokerGame: Insufficient balance");
userManagement.updateBalance(msg.sender, -int256(amount));
currentRound.playerBets[msg.sender] += amount;
currentRound.totalPot += amount;
emit PlayerBetPlaced(msg.sender, amount);
}
function distributePots() internal {
// ... existing code ...
for (uint256 i = 0; i < mainWinners.length; i++) {
userManagement.updateBalance(mainWinners[i], int256(mainPotShare));
emit PayoutProcessed(mainWinners[i], mainPotShare);
}
// ... handle side pots ...
}
}
@JW-dev0505 Game variables, pls update your contract! thank you!
## Unit Tests for Skia Poker Contracts
### Setup Instructions
1. **Install Necessary Dependencies:**
Open your terminal and run the following command to install the required dependencies for Hardhat, Ethers.js, Waffle, and Chai:
```bash
npm install --save-dev @nomiclabs/hardhat-ethers ethers @nomiclabs/hardhat-waffle ethereum-waffle chai
Create the Test File:
In your project's test
directory, create a new file named skia-poker.test.js
and add the following content:
const { expect } = require("chai");
const { ethers } = require("hardhat");
describe("Skia Poker", function () {
let GameManagement, RoomManagement, UserManagement, BettingAndPotManagement;
let gameManagement, roomManagement, userManagement, bettingAndPotManagement;
let owner, addr1, addr2;
beforeEach(async function () {
// Get the ContractFactory and Signers here.
GameManagement = await ethers.getContractFactory("GameManagement");
RoomManagement = await ethers.getContractFactory("RoomManagement");
UserManagement = await ethers.getContractFactory("UserManagement");
BettingAndPotManagement = await ethers.getContractFactory("BettingAndPotManagement");
[owner, addr1, addr2] = await ethers.getSigners();
// Deploy the contracts
gameManagement = await GameManagement.deploy();
roomManagement = await RoomManagement.deploy();
userManagement = await UserManagement.deploy();
bettingAndPotManagement = await BettingAndPotManagement.deploy(
1, // roomId
ethers.constants.AddressZero, // cardManagementAddress (replace with actual address)
roomManagement.address,
userManagement.address,
ethers.utils.parseEther("0.1"), // minimumBet
ethers.constants.AddressZero, // vrfCoordinator (replace with actual address)
ethers.constants.AddressZero, // linkToken (replace with actual address)
ethers.utils.formatBytes32String("keyhash"), // keyHash
ethers.utils.parseEther("0.1") // fee
);
// Grant necessary roles
await gameManagement.grantAdminRole(owner.address);
await userManagement.grantGameContractRole(bettingAndPotManagement.address);
});
describe("GameManagement", function () {
it("Should create a game room", async function () {
await gameManagement.createGameRoom(ethers.utils.parseEther("1"), 6);
const gameRoom = await gameManagement.getGameRoom(0);
expect(gameRoom.buyInAmount).to.equal(ethers.utils.parseEther("1"));
expect(gameRoom.maxPlayers).to.equal(6);
});
});
describe("RoomManagement", function () {
it("Should create a room", async function () {
await roomManagement.createRoom(1, ethers.utils.parseEther("1"));
const buyInAmount = await roomManagement.getBuyInAmount(1);
expect(buyInAmount).to.equal(ethers.utils.parseEther("1"));
});
});
describe("UserManagement", function () {
it("Should register a user", async function () {
await userManagement.connect(addr1).registerUser("Player1", { value: ethers.utils.parseEther("1") });
const balance = await userManagement.getUserBalance(addr1.address);
expect(balance).to.equal(ethers.utils.parseEther("1"));
});
});
describe("BettingAndPotManagement", function () {
it("Should allow a player to join the game", async function () {
await userManagement.connect(addr1).registerUser("Player1", { value: ethers.utils.parseEther("1") });
await bettingAndPotManagement.connect(addr1).joinGame({ value: ethers.utils.parseEther("1") });
const players = await bettingAndPotManagement.getPlayers();
expect(players).to.include(addr1.address);
});
});
});
Ensure that your hardhat.config.js
file is correctly set up for the Holesky testnet. Here is an example configuration:
require("@nomiclabs/hardhat-waffle");
module.exports = {
solidity: "0.8.19",
networks: {
holesky: {
url: "https://holesky.morphism.xyz", // Replace with the actual Morph RPC URL for Holesky
accounts: [process.env.PRIVATE_KEY], // Replace with your account's private key
},
},
};
To run the tests on the Holesky testnet, execute the following command in your terminal:
npx hardhat test --network holesky
These tests cover basic functionality such as creating game rooms, registering users, and joining games. Adjust the tests according to the specific implementations of your contracts and any additional features.
Security Reminder: Always use test accounts and test tokens when working with testnets. Never use real funds or private keys associated with real funds when testing.
@JW-dev0505
[SP-3] Develop PlayerManagement smart contract
Ensure security and gas efficiency in contract operations.
Jira Ticket [SP-3]: Develop PlayerManagement Smart Contract
Summary:
Develop the PlayerManagement smart contract for Skia Poker, which will handle essential functions such as player registration, balance management, and payouts. The contract must be optimized for security and gas efficiency to ensure a reliable and cost-effective gaming experience.
Description:
The PlayerManagement smart contract is a crucial component of the Skia Poker platform, responsible for managing player accounts, tracking balances, and handling payouts. This task involves implementing secure and efficient methods for player registration, updating balances during and after games, and processing payouts. The contract must be thoroughly tested to ensure robustness, security, and optimal gas usage.
Acceptance Criteria:
Tasks:
onlyOwner
,onlyRegisteredPlayer
) to restrict functions to authorized users.nonReentrant
modifier) to protect against reentrancy attacks.Priority: High
Assignee: James/Pete
Due Date: [Set a due date]
Dependencies:
Risks and Mitigations: