Skip to content

add: update ipfs by manager + new registerapp #302

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

Merged
sirpy merged 1 commit into from
Aug 10, 2025
Merged

add: update ipfs by manager + new registerapp #302

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
14 changes: 9 additions & 5 deletions packages/contracts/contracts/DirectPayments/DirectPaymentsFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,18 @@ pragma solidity >=0.8.0;
// import the DirectPayments contract
import "./DirectPaymentsPool.sol";
import "./ProvableNFT.sol";
import "../GoodCollective/SuperAppBaseFlow.sol";
import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol";
import "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol";

import { AccessControlUpgradeable } from "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

// import "hardhat/console.sol";

contract DirectPaymentsFactory is AccessControlUpgradeable, UUPSUpgradeable {
error NOT_PROJECT_OWNER();
error NOT_PROJECT_MANAGER();
error NOT_POOL();

event PoolCreated(
Expand Down Expand Up @@ -60,9 +61,9 @@ contract DirectPaymentsFactory is AccessControlUpgradeable, UUPSUpgradeable {
_;
}

modifier onlyPoolOwner(DirectPaymentsPool pool) {
if (pool.hasRole(pool.DEFAULT_ADMIN_ROLE(), msg.sender) == false) {
revert NOT_PROJECT_OWNER();
modifier onlyPoolManager(DirectPaymentsPool pool) {
if (pool.hasRole(pool.MANAGER_ROLE(), msg.sender) == false) {
revert NOT_PROJECT_MANAGER();
}

_;
Expand Down Expand Up @@ -136,6 +137,9 @@ contract DirectPaymentsFactory is AccessControlUpgradeable, UUPSUpgradeable {
pool = DirectPaymentsPool(address(new ERC1967Proxy(impl.implementation(), initCall)));
}

// Register the app with the host
IRegisterSuperapp(address(pool.host())).registerApp(address(pool), SuperAppDefinitions.APP_LEVEL_FINAL);

nft.grantRole(nft.getManagerRole(nextNftType), address(pool));

//access control to project is determinted by the first pool access control rules
Expand All @@ -153,7 +157,7 @@ contract DirectPaymentsFactory is AccessControlUpgradeable, UUPSUpgradeable {
nextNftType++;
}

function changePoolDetails(DirectPaymentsPool _pool, string memory _ipfs) external onlyPoolOwner(_pool) {
function changePoolDetails(DirectPaymentsPool _pool, string memory _ipfs) external onlyPoolManager(_pool) {
registry[address(_pool)].ipfs = _ipfs;
emit PoolDetailsChanged(address(_pool), _ipfs);
}
Expand Down
10 changes: 5 additions & 5 deletions packages/contracts/contracts/GoodCollective/GoodCollectiveSuperApp.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
pragma solidity >=0.8.0;

import { SuperAppBaseFlow } from "./SuperAppBaseFlow.sol";
import { ISuperfluid, ISuperToken, SuperAppDefinitions } from "@superfluid-finance/ethereum-contracts/contracts/interfaces/superfluid/ISuperfluid.sol";
import { ISuperfluid, ISuperToken, SuperAppDefinitions, ISuperApp } from "@superfluid-finance/ethereum-contracts/contracts/interfaces/superfluid/ISuperfluid.sol";
import { ISuperGoodDollar } from "@gooddollar/goodprotocol/contracts/token/superfluid/ISuperGoodDollar.sol";
import { SuperTokenV1Library } from "@superfluid-finance/ethereum-contracts/contracts/apps/SuperTokenV1Library.sol";
import { CFAv1Library, IConstantFlowAgreementV1 } from "@superfluid-finance/ethereum-contracts/contracts/apps/CFAv1Library.sol";
Expand Down Expand Up @@ -90,11 +90,11 @@ abstract contract GoodCollectiveSuperApp is SuperAppBaseFlow {
// Set the super token address
superToken = _superToken;

// Define the callback definitions for the app
Copy link

@sourcery-ai sourcery-ai bot Aug 10, 2025

Choose a reason for hiding this comment

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

nitpick: Commented-out registration logic may cause confusion.

If registration is managed elsewhere, please remove these commented lines or add a brief note clarifying the change.

uint256 callBackDefinitions = SuperAppDefinitions.APP_LEVEL_FINAL;
// // Define the callback definitions for the app
// uint256 callBackDefinitions = SuperAppDefinitions.APP_LEVEL_FINAL;

// Register the app with the host
host.registerApp(callBackDefinitions);
// // Register the app with the host
// host.registerApp(callBackDefinitions);

//initialize InitData struct, and set equal to cfaV1
cfaV1 = CFAv1Library.InitData(
Expand Down
4 changes: 4 additions & 0 deletions packages/contracts/contracts/GoodCollective/SuperAppBaseFlow.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ import { SuperTokenV1Library } from "@superfluid-finance/ethereum-contracts/cont

// import "hardhat/console.sol";

interface IRegisterSuperapp {
function registerApp(address app, uint256 configWord) external;
}

abstract contract SuperAppBaseFlow is ISuperApp {
using SuperTokenV1Library for ISuperToken;

Expand Down
12 changes: 9 additions & 3 deletions packages/contracts/contracts/UBI/UBIPoolFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@ import { AccessControlUpgradeable } from "@openzeppelin/contracts-upgradeable/ac
import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

import "../Interfaces.sol";
import "../GoodCollective/GoodCollectiveSuperApp.sol";
import "../GoodCollective/SuperAppBaseFlow.sol";

// import "hardhat/console.sol";

contract UBIPoolFactory is AccessControlUpgradeable, UUPSUpgradeable {
error NOT_PROJECT_OWNER();
error NOT_PROJECT_MANAGER();
Copy link

@sourcery-ai sourcery-ai bot Aug 10, 2025

Choose a reason for hiding this comment

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

suggestion: Review error usage for consistency with new access control.

Update all revert statements and error handling to use NOT_PROJECT_MANAGER where appropriate, and remove outdated errors that are no longer relevant to the new access control logic.

Suggested implementation:

    error NOT_PROJECT_MANAGER();
    error NOT_POOL();


        if (msg.sender != projectManager) {
            revert NOT_PROJECT_MANAGER();
        }


        require(hasRole(PROJECT_MANAGER_ROLE, msg.sender), "Not project manager");


        if (!isPool(msg.sender)) {
            revert NOT_POOL();
        }
  • You will need to update any references to projectOwner to projectManager if the access control logic has changed.
  • Ensure that the role constants (PROJECT_MANAGER_ROLE) are defined and used consistently throughout the contract.
  • Remove any other outdated error definitions and usages related to project ownership that are no longer relevant.
  • Review the rest of the contract for any additional revert statements or require checks that should be updated to use the new error and access control logic.

error NOT_POOL();

event PoolCreated(
Expand Down Expand Up @@ -58,8 +61,8 @@ contract UBIPoolFactory is AccessControlUpgradeable, UUPSUpgradeable {
_;
}

modifier onlyPoolOwner(UBIPool pool) {
if (pool.hasRole(pool.DEFAULT_ADMIN_ROLE(), msg.sender) == false) {
modifier onlyPoolManager(UBIPool pool) {
if (pool.hasRole(pool.MANAGER_ROLE(), msg.sender) == false) {
revert NOT_PROJECT_OWNER();
}

Expand Down Expand Up @@ -121,6 +124,9 @@ contract UBIPoolFactory is AccessControlUpgradeable, UUPSUpgradeable {
pool = UBIPool(address(new ERC1967Proxy(impl.implementation(), initCall)));
}

// Register the app with the host
IRegisterSuperapp(address(pool.host())).registerApp(address(pool), SuperAppDefinitions.APP_LEVEL_FINAL);

//access control to project is determinted by the first pool access control rules
if (address(projectIdToControlPool[keccak256(bytes(_projectId))]) == address(0))
projectIdToControlPool[keccak256(bytes(_projectId))] = pool;
Expand All @@ -134,7 +140,7 @@ contract UBIPoolFactory is AccessControlUpgradeable, UUPSUpgradeable {
emit PoolCreated(address(pool), _projectId, _ipfs, _settings, _limits);
}

function changePoolDetails(UBIPool _pool, string memory _ipfs) external onlyPoolOwner(_pool) {
function changePoolDetails(UBIPool _pool, string memory _ipfs) external onlyPoolManager(_pool) {
registry[address(_pool)].ipfs = _ipfs;
emit PoolDetailsChanged(address(_pool), _ipfs);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/contracts/hardhat.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ const config: HardhatUserConfig = {
'production-celo': {
chainId: 42220,
url: `https://forno.celo.org`,
gasPrice: 5000000000,
gasPrice: 25.1e9,
Copy link

@korbit-ai korbit-ai bot Aug 10, 2025

Choose a reason for hiding this comment

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

Excessive Gas Price Increase category Functionality

Tell me more
What is the issue?

The gas price modification from 5000000000 to 25.1e9 for Celo networks represents a significant increase (about 5x) which could lead to unnecessarily expensive transactions.

Why this matters

Higher gas prices will result in increased transaction costs for users, potentially making the system prohibitively expensive to use. The original gas price was already sufficient for most Celo transactions.

Suggested change ∙ Feature Preview

Maintain the previous gas price or use a more moderate increase. Example:

gasPrice: 10000000000, // 10 gwei
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

accounts: [privateKey],
verify: {
etherscan: {
Expand All @@ -94,7 +94,7 @@ const config: HardhatUserConfig = {
'development-celo': {
chainId: 42220,
url: `https://forno.celo.org`,
gasPrice: 5000000000,
gasPrice: 25.1e9,
accounts: {
mnemonic,
},
Expand Down
Loading