Skip to content
This repository has been archived by the owner on Oct 6, 2023. It is now read-only.

Commit

Permalink
Remove maxGeneralCategoryId & earlyLockedWithdrawFee from Accounts; U…
Browse files Browse the repository at this point in the history
…se Registrar Fee lookup for Charity Easrly Withdraw Fee in Accounts; Change one missed meta>>metadata;
  • Loading branch information
Andrey authored and Andrey committed Jul 25, 2023
1 parent 91e3396 commit 11968f3
Show file tree
Hide file tree
Showing 25 changed files with 39 additions and 108 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,6 @@ contract DiamondInit {
state.config.registrarContract = registrar;
state.config.networkName = "Polygon";
state.config.nextAccountId = 1;
state.config.maxGeneralCategoryId = 1;
state.config.earlyLockedWithdrawFee = LibAccounts.FeeSetting({
payoutAddress: address(0),
bps: 1000 // 10% fee placeholder. Can always change later if needed
});

// add your own state variables
// EIP-2535 specifies that the `diamondCut` function takes two optional
Expand Down
2 changes: 1 addition & 1 deletion contracts/core/accounts/facets/AccountsCreateEndowment.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ contract AccountsCreateEndowment is
address registrarAddress = state.config.registrarContract;

RegistrarStorage.Config memory registrar_config = IRegistrar(registrarAddress).queryConfig();
LibAccounts.FeeSetting memory earlyLockedWithdrawFee = IRegistrar(registrarAddress).getFeeSettingsByFeeType(LibAccounts.FeeTypes.EarlyLockedWithdrawCharity);

LibAccounts.FeeSetting memory earlyLockedWithdrawFee = state.config.earlyLockedWithdrawFee;
if (LibAccounts.EndowmentType.Charity == details.endowType) {
require(msg.sender == registrar_config.charityApplications, "Unauthorized");
} else {
Expand Down
4 changes: 1 addition & 3 deletions contracts/core/accounts/facets/AccountsQueryEndowments.sol
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,7 @@ contract AccountsQueryEndowments is IAccountsQueryEndowments {
version: state.config.version,
networkName: state.config.networkName,
registrarContract: state.config.registrarContract,
nextAccountId: state.config.nextAccountId,
maxGeneralCategoryId: state.config.maxGeneralCategoryId,
earlyLockedWithdrawFee: state.config.earlyLockedWithdrawFee
nextAccountId: state.config.nextAccountId
});
}

Expand Down
9 changes: 1 addition & 8 deletions contracts/core/accounts/facets/AccountsUpdate.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,14 @@ contract AccountsUpdate is ReentrancyGuardFacet, IAccountsEvents, IAccountsUpdat
* @notice This function updates the config of the contract
* @dev This function updates the config of the contract
* @param newRegistrar The new registrar contract
* @param maxGeneralCategoryId The max general category id
*/
function updateConfig(
address newRegistrar,
uint256 maxGeneralCategoryId,
LibAccounts.FeeSetting memory earlyLockedWithdrawFee
) public nonReentrant {
function updateConfig(address newRegistrar) public nonReentrant {
AccountStorage.State storage state = LibAccounts.diamondStorage();

require(msg.sender == state.config.owner, "Unauthorized");
require(Validator.addressChecker(newRegistrar), "invalid registrar address");

state.config.registrarContract = newRegistrar;
state.config.maxGeneralCategoryId = maxGeneralCategoryId;
state.config.earlyLockedWithdrawFee = earlyLockedWithdrawFee;

emit ConfigUpdated();
}
Expand Down
7 changes: 1 addition & 6 deletions contracts/core/accounts/interfaces/IAccountsUpdate.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ interface IAccountsUpdate {
* @notice This function updates the config of the contract
* @dev This function updates the config of the contract
* @param newRegistrar The new registrar contract
* @param maxGeneralCategoryId The max general category id
*/
function updateConfig(
address newRegistrar,
uint256 maxGeneralCategoryId,
LibAccounts.FeeSetting memory earlyLockedWithdrawFee
) external;
function updateConfig(address newRegistrar) external;
}
1 change: 0 additions & 1 deletion contracts/core/accounts/message.sol
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ library AccountMessages {
string networkName;
address registrarContract;
uint256 nextAccountId;
uint256 maxGeneralCategoryId;
}

struct StateResponse {
Expand Down
1 change: 0 additions & 1 deletion contracts/core/accounts/storage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ library AccountStorage {
string networkName;
address registrarContract;
uint32 nextAccountId;
uint256 maxGeneralCategoryId;
bool reentrancyGuardLocked;
}

Expand Down
2 changes: 1 addition & 1 deletion contracts/multisigs/CharityApplications.sol
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ contract CharityApplications is MultiSigGeneric, StorageApplications, ICharityAp
proposals[proposalCount] = ApplicationsStorage.ApplicationProposal({
proposer: msg.sender,
application: _application,
meta: _metadata,
metadata: _metadata,
expiry: expiry,
executed: false
});
Expand Down
9 changes: 4 additions & 5 deletions tasks/helpers/updateRegistrar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,13 @@ export async function updateRegistrarNetworkConnections(
) {
logger.divider();

let networkName
let networkName;
try {

// If we're updating info on this chain for another chain, arg info MUST specify chain id
// If we're updating info on this chain for another chain, arg info MUST specify chain id
if (Number(networkInfo.chainId) > 0) {
networkName = getNetworkNameFromChainId(Number(networkInfo.chainId));
}
else { // we're updating this chains own network info and can safely lookup chain id
} else {
// we're updating this chains own network info and can safely lookup chain id
const chainId = await getChainId(hre);
networkName = getNetworkNameFromChainId(chainId);
}
Expand Down
25 changes: 1 addition & 24 deletions tasks/manage/accounts/updateConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,11 @@ import {
import {confirmAction, getAddresses, getSigners, logger} from "utils";

type TaskArgs = {
earlyLockedWithdrawFeeBps?: number;
earlyLockedWithdrawFeePayoutAddress?: string;
maxGeneralCategoryId?: number;
registrarContract?: string;
yes: boolean;
};

task("manage:accounts:updateConfig", "Will update Accounts Diamond config")
.addOptionalParam(
"earlyLockedWithdrawFeeBps",
"Early locked withdraw fee BPS.",
undefined,
types.int
)
.addOptionalParam(
"earlyLockedWithdrawFeePayoutAddress",
"Early locked withdraw fee payout address."
)
.addOptionalParam("maxGeneralCategoryId", "The max general category id.", undefined, types.int)
.addOptionalParam(
"registrarContract",
"Registrar contract address. Will do a local lookup from contract-address.json if none is provided."
Expand All @@ -44,9 +30,7 @@ task("manage:accounts:updateConfig", "Will update Accounts Diamond config")
addresses.accounts.diamond,
apTeamMultisigOwners[0]
);
const {registrarContract, earlyLockedWithdrawFee, maxGeneralCategoryId, owner} =
await accountsQueryEndowments.queryConfig();
const curConfig = {registrarContract, earlyLockedWithdrawFee, maxGeneralCategoryId};
const {registrarContract, owner} = await accountsQueryEndowments.queryConfig();
logger.out(curConfig);

Check failure on line 34 in tasks/manage/accounts/updateConfig.ts

View workflow job for this annotation

GitHub Actions / ci

Cannot find name 'curConfig'.

logger.out("Config data to update:");
Expand All @@ -64,13 +48,6 @@ task("manage:accounts:updateConfig", "Will update Accounts Diamond config")
);
const data = accountsUpdate.interface.encodeFunctionData("updateConfig", [
newConfig.registrarContract || curConfig.registrarContract,

Check failure on line 50 in tasks/manage/accounts/updateConfig.ts

View workflow job for this annotation

GitHub Actions / ci

Cannot find name 'curConfig'.
newConfig.maxGeneralCategoryId || curConfig.maxGeneralCategoryId,
{
bps: newConfig.earlyLockedWithdrawFeeBps || curConfig.earlyLockedWithdrawFee.bps,
payoutAddress:
newConfig.earlyLockedWithdrawFeePayoutAddress ||
curConfig.earlyLockedWithdrawFee.payoutAddress,
},
]);
const apTeamMultiSig = APTeamMultiSig__factory.connect(
owner, // ensure connection to current owning APTeamMultiSig contract
Expand Down
2 changes: 1 addition & 1 deletion tasks/manage/queryEndowments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ task("manage:queryEndowments", "Will create a new endowment")
apTeam1
);

logger.out(await queryEndowmentFacet.queryEndowmentDetails(taskArgs.id))
logger.out(await queryEndowmentFacet.queryEndowmentDetails(taskArgs.id));
} catch (error) {
logger.out(error, logger.Level.Error);
}
Expand Down
7 changes: 5 additions & 2 deletions tasks/manage/registrar/setNetworkInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@ task("manage:registrar:setNetworkInfo", "Set network info for a specified networ
logger.out("Connecting to registrar on specified network...");

const networkId = getChainIdFromNetworkName(taskArguments.networkName);
const thisNetworkAddresses = await getAddresses(hre)
const thatNetworkAddresses = getAddressesByNetworkId(networkId, DEFAULT_CONTRACT_ADDRESS_FILE_PATH);
const thisNetworkAddresses = await getAddresses(hre);
const thatNetworkAddresses = getAddressesByNetworkId(
networkId,
DEFAULT_CONTRACT_ADDRESS_FILE_PATH
);
const newNetworkInfo: Partial<IAccountsStrategy.NetworkInfoStruct> = {
chainId: networkId,
router: thatNetworkAddresses.router.proxy,
Expand Down
5 changes: 4 additions & 1 deletion tasks/manage/registrar/setStratParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ task("manage:registrar:setStratParams")
logger.out("Connecting to registrar on specified network...");
const addresses = await getAddresses(hre);
const {apTeamMultisigOwners} = await getSigners(hre);
const registrar = Registrar__factory.connect(addresses.registrar.proxy, apTeamMultisigOwners[0]);
const registrar = Registrar__factory.connect(
addresses.registrar.proxy,
apTeamMultisigOwners[0]
);
logger.pad(50, "Connected to Registrar at: ", registrar.address);

logger.divider();
Expand Down
5 changes: 3 additions & 2 deletions tasks/upgrade/upgradeRegistrar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ task("upgrade:registrar", "Will upgrade the Registrar (use only on the primary c
.addFlag("yes", "Automatic yes to prompt.")
.setAction(async (taskArgs: {skipVerify: boolean; yes: boolean}, hre) => {
try {
const isConfirmed = taskArgs.yes || (await confirmAction("Upgrading Registrar implementation..."));
const isConfirmed =
taskArgs.yes || (await confirmAction("Upgrading Registrar implementation..."));
if (!isConfirmed) {
return logger.out("Confirmation denied.", logger.Level.Warn);
}
Expand All @@ -42,7 +43,7 @@ task("upgrade:registrar", "Will upgrade the Registrar (use only on the primary c
await updateAddresses(
{
registrar: {
implementation: registrar.address
implementation: registrar.address,
},
},
hre
Expand Down
2 changes: 0 additions & 2 deletions test/core/accounts/AccountsCreateEndowment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,6 @@ describe("AccountsCreateEndowment", function () {
networkName: "Polygon",
registrarContract: registrarFake.address,
nextAccountId: expectedNextAccountId,
maxGeneralCategoryId: 1,
earlyLockedWithdrawFee: {bps: 1000, payoutAddress: ethers.constants.AddressZero},
reentrancyGuardLocked: false,
});

Expand Down
2 changes: 0 additions & 2 deletions test/core/accounts/AccountsDeployContract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ describe("AccountsDeployContract", function () {
networkName: "Polygon",
registrarContract: registrarFake.address,
nextAccountId: 1,
maxGeneralCategoryId: 1,
earlyLockedWithdrawFee: {bps: 1000, payoutAddress: ethers.constants.AddressZero},
reentrancyGuardLocked: false,
});

Expand Down
2 changes: 0 additions & 2 deletions test/core/accounts/AccountsDepositWithdrawEndowments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,6 @@ describe("AccountsDepositWithdrawEndowments", function () {
networkName: "Polygon",
registrarContract: registrarFake.address,
nextAccountId: 3, // 2 endows already added
maxGeneralCategoryId: 1,
earlyLockedWithdrawFee: {bps: 5, payoutAddress: ethers.constants.AddressZero},
reentrancyGuardLocked: false,
});
});
Expand Down
2 changes: 0 additions & 2 deletions test/core/accounts/AccountsDonationMatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,6 @@ describe("AccountsDonationMatch", function () {
networkName: "Polygon",
registrarContract: registrarFake.address,
nextAccountId: endowId + 1,
maxGeneralCategoryId: 1,
earlyLockedWithdrawFee: {bps: 1000, payoutAddress: ethers.constants.AddressZero},
reentrancyGuardLocked: false,
});
});
Expand Down
2 changes: 0 additions & 2 deletions test/core/accounts/AccountsQueryEndowments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,6 @@ describe("AccountsQueryEndowments", function () {
const configResponse = await facet.queryConfig();

// Assert the expected config
expect(configResponse.earlyLockedWithdrawFee).to.equalFee(config.earlyLockedWithdrawFee);
expect(configResponse.maxGeneralCategoryId).to.equal(config.maxGeneralCategoryId);
expect(configResponse.nextAccountId).to.equal(config.nextAccountId);
expect(configResponse.owner).to.equal(config.owner);
expect(configResponse.registrarContract).to.equal(config.registrarContract);
Expand Down
27 changes: 7 additions & 20 deletions test/core/accounts/AccountsUpdate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ describe("AccountsUpdate", function () {
let state: TestFacetProxyContract;

let newRegistrar: string;
let maxGeneralCategoryId: number;
let earlyLockedWithdrawFee: LibAccounts.FeeSettingStruct;

before(async function () {
const signers = await getSigners(hre);
Expand All @@ -27,11 +25,6 @@ describe("AccountsUpdate", function () {
user = signers.deployer;

newRegistrar = signers.airdropOwner.address;
maxGeneralCategoryId = 2;
earlyLockedWithdrawFee = {
bps: 2000,
payoutAddress: owner.address,
};
});

beforeEach(async function () {
Expand All @@ -45,8 +38,6 @@ describe("AccountsUpdate", function () {
networkName: "Polygon",
registrarContract: ethers.constants.AddressZero,
nextAccountId: 1,
maxGeneralCategoryId: 1,
earlyLockedWithdrawFee: {bps: 1000, payoutAddress: ethers.constants.AddressZero},
reentrancyGuardLocked: false,
});

Expand Down Expand Up @@ -81,29 +72,25 @@ describe("AccountsUpdate", function () {

describe("updateConfig", () => {
it("should update the config when called by the owner", async () => {
expect(
await facet.updateConfig(newRegistrar, maxGeneralCategoryId, earlyLockedWithdrawFee)
).to.emit(facet, "ConfigUpdated");
expect(await facet.updateConfig(newRegistrar)).to.emit(facet, "ConfigUpdated");

const config = await state.getConfig();

expect(config.registrarContract).to.equal(newRegistrar);
expect(config.maxGeneralCategoryId).to.equal(maxGeneralCategoryId);
expect(config.earlyLockedWithdrawFee).to.equalFee(earlyLockedWithdrawFee);
});

it("should revert when called by a non-owner address", async () => {
await expect(
facet.connect(user).updateConfig(newRegistrar, maxGeneralCategoryId, earlyLockedWithdrawFee)
).to.be.revertedWith("Unauthorized");
await expect(facet.connect(user).updateConfig(newRegistrar)).to.be.revertedWith(
"Unauthorized"
);
});

it("should revert when the registrar address is invalid", async () => {
const invalidAddress = ethers.constants.AddressZero;

await expect(
facet.updateConfig(invalidAddress, maxGeneralCategoryId, earlyLockedWithdrawFee)
).to.be.revertedWith("invalid registrar address");
await expect(facet.updateConfig(invalidAddress)).to.be.revertedWith(
"invalid registrar address"
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ describe("AccountsUpdateEndowmentSettingsController", function () {
networkName: "",
registrarContract: ethers.constants.AddressZero,
nextAccountId: 1,
maxGeneralCategoryId: 1,
earlyLockedWithdrawFee: {bps: 1000, payoutAddress: ethers.constants.AddressZero},
reentrancyGuardLocked: false,
});

Expand Down
2 changes: 0 additions & 2 deletions test/core/accounts/AccountsUpdateEndowments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -677,8 +677,6 @@ describe("AccountsUpdateEndowments", function () {
version: "1",
registrarContract: registrarFake.address,
nextAccountId: 1,
maxGeneralCategoryId: 1,
earlyLockedWithdrawFee: {bps: 1000, payoutAddress: ethers.constants.AddressZero},
reentrancyGuardLocked: false,
});
return registrarFake;
Expand Down
2 changes: 0 additions & 2 deletions test/core/accounts/AccountsUpdateStatusEndowments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,6 @@ describe("AccountsUpdateStatusEndowments", function () {
networkName: "Polygon",
registrarContract: registrarFake.address,
nextAccountId: accountId + 1,
maxGeneralCategoryId: 1,
earlyLockedWithdrawFee: {bps: 1000, payoutAddress: ethers.constants.AddressZero},
reentrancyGuardLocked: false,
});

Expand Down
Loading

0 comments on commit 11968f3

Please sign in to comment.