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.

Sign up for GitHub

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

feat: add params to IR factory #198

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add params to IR factory #198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions packages/zevm-app-contracts/contracts/instant-rewards/InstantRewardsFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import "@openzeppelin/contracts/access/Ownable2Step.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
import "./InstantRewardsV2.sol";

contract InstantRewardsFactory is Ownable2Step {
contract InstantRewardsFactory is Ownable {
bool public allowPublicCreation = false;

error AccessDenied();
Expand All @@ -13,7 +13,7 @@ contract InstantRewardsFactory is Ownable2Step {
error StartTimeInPast();
error EndTimeBeforeStart();

event InstantRewardsCreated(address indexed instantRewards, address indexed owner);
event InstantRewardsCreated(address indexed instantRewards, address indexed owner, string indexed name);

constructor(address owner) Ownable() {
transferOwnership(owner);
Expand All @@ -27,7 +27,10 @@ contract InstantRewardsFactory is Ownable2Step {
address signerAddress,
uint256 start,
uint256 end,
string memory name
string memory name,
string memory promoUrl,
string memory avatarUrl,
string memory description
Comment on lines +30 to +33
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider validating new string parameters to prevent empty values

The new parameters promoUrl, avatarUrl, and description are accepted without validation. To ensure data integrity, consider adding checks to validate that these strings are not empty and conform to expected formats.

Apply this diff to add validation:

+    if (bytes(promoUrl).length == 0) revert EmptyPromoUrl();
+    if (bytes(avatarUrl).length == 0) revert EmptyAvatarUrl();
+    if (bytes(description).length == 0) revert EmptyDescription();

Ensure to define the new error types accordingly.

Committable suggestion skipped: line range outside the PR's diff.

) external returns (address) {
if (signerAddress == address(0)) revert InvalidSignerAddress();
if (bytes(name).length == 0) revert EmptyName();
Expand All @@ -37,9 +40,18 @@ contract InstantRewardsFactory is Ownable2Step {
bool isOwner = owner() == msg.sender;
if (!allowPublicCreation && !isOwner) revert AccessDenied();

InstantRewardsV2 instantRewards = new InstantRewardsV2(signerAddress, owner(), start, end, name);
InstantRewardsV2 instantRewards = new InstantRewardsV2(
signerAddress,
owner(),
start,
end,
name,
promoUrl,
avatarUrl,
description
);
instantRewards.transferOwnership(owner());
emit InstantRewardsCreated(address(instantRewards), owner());
emit InstantRewardsCreated(address(instantRewards), owner(), name);
return address(instantRewards);
}
}
12 changes: 10 additions & 2 deletions packages/zevm-app-contracts/contracts/instant-rewards/InstantRewardsV2.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ contract InstantRewardsV2 is InstantRewards {

uint256 public start;
uint256 public end;
string public promoUrl;
string public avatarUrl;
string public description;
Comment on lines +11 to +13
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding length validation for metadata fields

The new public string variables could lead to excessive gas costs during deployment if the strings are too long. Consider adding length validation in the constructor.

+ error MetadataFieldTooLong();
+ uint256 private constant MAX_METADATA_LENGTH = 512; // Adjust as needed

  constructor(...) {
+     if (
+         bytes(promoUrl_).length > MAX_METADATA_LENGTH ||
+         bytes(avatarUrl_).length > MAX_METADATA_LENGTH ||
+         bytes(description_).length > MAX_METADATA_LENGTH
+     ) revert MetadataFieldTooLong();
      // existing code
  }

Committable suggestion skipped: line range outside the PR's diff.


event TimeframeUpdated(uint256 start, uint256 end);

Expand All @@ -20,13 +23,19 @@ contract InstantRewardsV2 is InstantRewards {
address owner,
uint256 start_,
uint256 end_,
string memory name_
string memory name_,
string memory promoUrl_,
string memory avatarUrl_,
string memory description_
) InstantRewards(signerAddress_, owner) {
if (signerAddress_ == address(0)) revert InvalidAddress();
if (start_ > end_) revert InvalidTimeframe();
start = start_;
end = end_;
name = name_;
promoUrl = promoUrl_;
avatarUrl = avatarUrl_;
description = description_;
}

function isActive() public view returns (bool) {
Expand All @@ -48,7 +57,6 @@ contract InstantRewardsV2 is InstantRewards {
}

function withdraw(address wallet, uint256 amount) public override onlyOwner {
if (isActive()) revert InstantRewardStillActive();
super.withdraw(wallet, amount);
}
}
5 changes: 3 additions & 2 deletions packages/zevm-app-contracts/data/addresses.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
"ProofOfLiveness": "0x981EB6fD19717Faf293Fba0cBD05C6Ac97b8C808",
"TimelockController": "0x44139C2150c11c25f517B8a8F974b59C82aEe709",
"ZetaXPGov": "0x854032d484aE21acC34F36324E55A8080F21Af12",
"invitationManagerV2": "0xb80f6360194Dd6B47B80bd8730b3dBb05a39e723"
"invitationManagerV2": "0xb80f6360194Dd6B47B80bd8730b3dBb05a39e723",
"InstantRewardsFactory": "0x02F054A5BeeB2653d1c2403cBe9B262356fdD5E4"
},
"zeta_mainnet": {
"disperse": "0x23ce409Ea60c3d75827d04D9db3d52F3af62e44d",
Expand All @@ -22,7 +23,7 @@
"invitationManager": "0x3649C03C472B698213926543456E9c21081e529d",
"withdrawERC20": "0xa349B9367cc54b47CAb8D09A95836AE8b4D1d84E",
"ZetaXP": "0x9A4e8bB5FFD8088ecF1DdE823e97Be8080BD38cb",
"InstantRewards": "0x018412ec1D5bBb864eAe0A4BECaa683052890238",
"InstantRewards": "0xfD5dcBf68c81274B48593Cb4b0322e965741392b",
"ProofOfLiveness": "0x327c9837B183e69C522a30E6f91A42c86e057432"
}
}
Expand Down
11 changes: 9 additions & 2 deletions packages/zevm-app-contracts/hardhat.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ const config: HardhatUserConfig = {
chainId: 7001,
network: "zeta_testnet",
urls: {
apiURL: "https://zetachain-athens-3.blockscout.com/api",
browserURL: "https://zetachain-athens-3.blockscout.com",
apiURL: "https://zetachain-testnet.blockscout.com/api",
browserURL: "https://zetachain-testnet.blockscout.com",
},
},
],
Expand All @@ -52,6 +52,13 @@ const config: HardhatUserConfig = {
},
networks: {
...getHardhatConfigNetworks(),
zeta_mainnet: {
accounts: PRIVATE_KEYS,
chainId: 7000,
gas: "auto",
gasMultiplier: 3,
url: `https://zetachain-evm.blockpi.network/v1/rpc/public`,
},
},
solidity: {
compilers: [
Expand Down
4 changes: 2 additions & 2 deletions packages/zevm-app-contracts/scripts/instant-rewards/deploy-v2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { verifyContract } from "../explorer.helpers";

const networkName = network.name;

const owner = "0x1d24d94520B94B26351f6573de5ef9731c48531A";
const owner = "0xD7E8bD37db625a4856E056D2617C9d140dB99182";

const deployInstantRewards = async () => {
if (!isProtocolNetworkName(networkName)) throw new Error("Invalid network name");
Expand All @@ -21,7 +21,7 @@ const deployInstantRewards = async () => {

console.log("InstantRewards deployed to:", InstantRewards.address);

saveAddress("InstantRewards", InstantRewards.address, networkName);
saveAddress("InstantRewardsFactory", InstantRewards.address, networkName);

await verifyContract(InstantRewards.address, [owner]);
};
Expand Down
16 changes: 12 additions & 4 deletions packages/zevm-app-contracts/test/instant-rewards/instant-rewards-v2-compatibility.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,16 @@ describe("Instant Rewards V2 Contract Compatibility test", () => {
const start = (await ethers.provider.getBlock("latest")).timestamp + 1;
const end = start + 1000;

instantRewards = await instantRewardsFactory.deploy(signer.address, owner.address, start, end, "Instant Rewards");
instantRewards = await instantRewardsFactory.deploy(
signer.address,
owner.address,
start,
end,
"Instant Rewards",
"http://img.com",
"http://avatar.com",
"Description"
);

await instantRewards.deployed();
});
Expand Down Expand Up @@ -322,7 +331,7 @@ describe("Instant Rewards V2 Contract Compatibility test", () => {
expect(balanceOfUser).to.be.eq(userBalanceBefore.add(amountToWithdraw));
});

it("Should fail if try to withdraw an active IR", async () => {
it("Should be able to withdraw an active IR", async () => {
const amount = utils.parseEther("2");
const amountToWithdraw = utils.parseEther("1");
// transfer some funds to the contract
Expand All @@ -331,7 +340,6 @@ describe("Instant Rewards V2 Contract Compatibility test", () => {
value: amount,
});

const tx = instantRewards.withdraw(user.address, amountToWithdraw);
await expect(tx).to.revertedWith("InstantRewardStillActive");
await instantRewards.withdraw(user.address, amountToWithdraw);
});
});
25 changes: 21 additions & 4 deletions packages/zevm-app-contracts/test/instant-rewards/instant-rewards-v2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,16 @@ describe("Instant Rewards Contract test", () => {
const instantRewardsFactoryF = await ethers.getContractFactory("InstantRewardsFactory");
instantRewardsFactory = (await instantRewardsFactoryF.deploy(owner.address)) as InstantRewardsFactory;
await instantRewardsFactory.deployed();
await instantRewardsFactory.connect(owner).acceptOwnership();
});

it("Should deploy an IR instance", async () => {
const currentBlock = await ethers.provider.getBlock("latest");
const start = currentBlock.timestamp + 1000;
const end = start + 1000;
const name = "Instant Rewards";
const tx = instantRewardsFactory.connect(owner).createInstantRewards(signer.address, start, end, name);
const tx = instantRewardsFactory
.connect(owner)
.createInstantRewards(signer.address, start, end, name, "http://img.com", "http://avatar.com", "Description");
Comment on lines +27 to +29
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage for metadata fields

The current tests only verify happy path scenarios with hardcoded values. Consider adding test cases for:

  • Empty strings
  • Maximum length validation
  • Special characters in URLs
  • Invalid URL formats

Example test structure:

it("Should revert when metadata exceeds maximum length", async () => {
  const longString = "a".repeat(1000);
  await expect(
    instantRewardsFactory.createInstantRewards(
      signer.address,
      start,
      end,
      name,
      longString,
      "http://avatar.com",
      "Description"
    )
  ).to.be.revertedWith("MetadataFieldTooLong");
});

Also applies to: 51-59, 70-78

await expect(tx).to.emit(instantRewardsFactory, "InstantRewardsCreated");

const events = await instantRewardsFactory.queryFilter("InstantRewardsCreated");
Expand All @@ -47,7 +48,15 @@ describe("Instant Rewards Contract test", () => {
const start = currentBlock.timestamp + 1000;
const end = start + 1000;
const name = "Instant Rewards";
const tx = instantRewardsFactory.createInstantRewards(signer.address, start, end, name);
const tx = instantRewardsFactory.createInstantRewards(
signer.address,
start,
end,
name,
"http://img.com",
"http://avatar.com",
"Description"
);
await expect(tx).to.revertedWith("AccessDenied");
});

Expand All @@ -58,7 +67,15 @@ describe("Instant Rewards Contract test", () => {
const start = currentBlock.timestamp + 1000;
const end = start + 1000;
const name = "Instant Rewards";
const tx = instantRewardsFactory.createInstantRewards(signer.address, start, end, name);
const tx = instantRewardsFactory.createInstantRewards(
signer.address,
start,
end,
name,
"http://img.com",
"http://avatar.com",
"Description"
);
await expect(tx).to.emit(instantRewardsFactory, "InstantRewardsCreated");
});
});
Loading