Skip to content

Commit

Permalink
fix comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@josemarinas
josemarinas committed Nov 22, 2023
1 parent 1dba061 commit a58c9f7
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 45 deletions.
14 changes: 12 additions & 2 deletions modules/client/src/internal/client/methods.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1231,9 +1231,19 @@ export class ClientMethods extends ClientCore implements IClientMethods {
if (!iproposal) {
return {
isValid: false,
causes: [
DaoUpdateProposalInvalidityCause.PROPOSAL_NOT_FOUND,
proposalSettingsErrorCauses: [
ProposalSettingsErrorCause.PROPOSAL_NOT_FOUND,
],
actionErrorCauses: [],
};
}
// check failure map
if (iproposal.allowFailureMap !== "0") {
// if the failure map is not 0 return invalid failure map
return {
isValid: false,
actionErrorCauses: [],
proposalSettingsErrorCauses: [ProposalSettingsErrorCause.NON_ZERO_ALLOW_FAILURE_MAP_VALUE]
};
}
// get implementation address, use latest version as default
Expand Down
4 changes: 2 additions & 2 deletions modules/client/src/internal/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,12 @@ export const UPDATE_PLUGIN_SIGNATURES: string[] = [
];

export const PLUGIN_UPDATE_ACTION_PATTERN = [
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_PLUGIN_UPGRADE_PERMISSION,
];
export const PLUGIN_UPDATE_WITH_ROOT_ACTION_PATTERN = [
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.GRANT_ROOT_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_ROOT_PERMISSION,
Expand Down
4 changes: 2 additions & 2 deletions modules/client/src/internal/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,8 @@ export enum ProposalActionTypes {
UPGRADE_TO = "upgradeTo",
UPGRADE_TO_AND_CALL = "upgradeToAndCall",
APPLY_UPDATE = "applyUpdate",
GRANT_PLUGIN_UPDATE_PERMISSION = "grantUpdatePluginPermission",
REVOKE_PLUGIN_UPGRADE_PERMISSION = "revokeUpdatePluginPermission",
GRANT_PLUGIN_UPGRADE_PERMISSION = "grantUpgradePluginPermission",
REVOKE_PLUGIN_UPGRADE_PERMISSION = "revokeUpgradePluginPermission",
GRANT_ROOT_PERMISSION = "grantRootPermission",
REVOKE_ROOT_PERMISSION = "revokeRootPermission",
ACTION_NOT_ALLOWED = "actionNotAllowed",
Expand Down
53 changes: 32 additions & 21 deletions modules/client/src/internal/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1190,7 +1190,7 @@ export async function validateApplyUpdateFunction(
causes.push(
PluginUpdateProposalInValidityCause.MISSING_PLUGIN_PREPARATION,
);
return causes
return causes;
}
// get the metadata of the plugin repo
// for the release and build specified
Expand Down Expand Up @@ -1272,7 +1272,7 @@ export function classifyProposalActions(
switch (decodedPermission.permission) {
case Permissions.UPGRADE_PLUGIN_PERMISSION:
classifiedActions.push(
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
);
break;
case Permissions.ROOT_PERMISSION:
Expand Down Expand Up @@ -1347,7 +1347,9 @@ export function containsPluginUpdateActionBlockWithRootPermission(
* @param {ProposalActionTypes[]} actions
* @return {*} {boolean}
*/
export function containsPluginUpdateActionBlock(actions: ProposalActionTypes[]): boolean {
export function containsPluginUpdateActionBlock(
actions: ProposalActionTypes[],
): boolean {
// get the first 3 action
const receivedPattern = actions.slice(0, 3);
// check if it matches the expected pattern
Expand All @@ -1362,7 +1364,9 @@ export function containsPluginUpdateActionBlock(actions: ProposalActionTypes[]):
* @param {ProposalActionTypes[]} actions
* @return {*} {boolean}
*/
export function containsDaoUpdateAction(actions: ProposalActionTypes[]): boolean {
export function containsDaoUpdateAction(
actions: ProposalActionTypes[],
): boolean {
// UpgradeTo or UpgradeToAndCall should be the first action
return actions[0] === ProposalActionTypes.UPGRADE_TO ||
actions[0] === ProposalActionTypes.UPGRADE_TO_AND_CALL;
Expand All @@ -1375,31 +1379,33 @@ export function validateUpdateDaoProposalActions(
currentDaoVersion: [number, number, number],
): DaoUpdateProposalValidity {
const classifiedActions = classifyProposalActions(actions);
const causes: DaoUpdateProposalInvalidityCause[] = [];
const actionErrorCauses: DaoUpdateProposalInvalidityCause[] = [];
const proposalSettingsErrorCauses: ProposalSettingsErrorCause[] = [];
// check if the actions are valid
if (!containsDaoUpdateAction(classifiedActions)) {
// If actions are not valid, add the cause to the causes array
// and return
causes.push(
DaoUpdateProposalInvalidityCause.INVALID_ACTIONS,
);
return { isValid: false, causes };
return {
isValid: false,
proposalSettingsErrorCauses: [ProposalSettingsErrorCause.INVALID_ACTIONS],
actionErrorCauses: [],
};
}
// if they are valid, this means that
// if they are valid, this means that
// the upgrade action must be the first one
const upgradeActionType = classifiedActions[0];
const upgradeAction = actions[0];
// if the to address is not the dao address
// add the cause to the causes array
if (upgradeAction.to !== daoAddress) {
causes.push(
actionErrorCauses.push(
DaoUpdateProposalInvalidityCause.INVALID_TO_ADDRESS,
);
}
// if the value is different from 0
// add the cause to the causes array
if (upgradeAction.value.toString() !== "0") {
causes.push(
actionErrorCauses.push(
DaoUpdateProposalInvalidityCause.NON_ZERO_CALL_VALUE,
);
}
Expand All @@ -1411,7 +1417,7 @@ export function validateUpdateDaoProposalActions(
);
// check that the implementation address is the same
if (expectedImplementationAddress !== decodedImplementationAddress) {
causes.push(
actionErrorCauses.push(
DaoUpdateProposalInvalidityCause
.INVALID_UPGRADE_TO_IMPLEMENTATION_ADDRESS,
);
Expand All @@ -1432,7 +1438,7 @@ export function validateUpdateDaoProposalActions(
expectedImplementationAddress !==
upgradeToAndCallDecodedParams.implementationAddress
) {
causes.push(
actionErrorCauses.push(
DaoUpdateProposalInvalidityCause
.INVALID_UPGRADE_TO_AND_CALL_IMPLEMENTATION_ADDRESS,
);
Expand All @@ -1443,27 +1449,32 @@ export function validateUpdateDaoProposalActions(
JSON.stringify(initializeFromDecodedParams.previousVersion) !==
JSON.stringify(currentDaoVersion)
) {
causes.push(
actionErrorCauses.push(
DaoUpdateProposalInvalidityCause.INVALID_UPGRADE_TO_AND_CALL_VERSION,
);
}
// TODO
// check that the data can be decoded with the abi in the metadata of the version we are
// upgrading to. For now is not possible so we check that the data is empty
if (initializeFromDecodedParams.initData.length !== 0) {
causes.push(
actionErrorCauses.push(
DaoUpdateProposalInvalidityCause.INVALID_UPGRADE_TO_AND_CALL_DATA,
);
}
break;
default:
causes.push(
DaoUpdateProposalInvalidityCause.INVALID_ACTIONS,
proposalSettingsErrorCauses.push(
ProposalSettingsErrorCause.INVALID_ACTIONS,
);
break;
}
// return the validity of the proposal
return { isValid: causes.length === 0, causes };
return {
isValid: actionErrorCauses.length === 0 &&
proposalSettingsErrorCauses.length === 0,
actionErrorCauses,
proposalSettingsErrorCauses,
};
}

export async function validateUpdatePluginProposalActions(
Expand All @@ -1490,7 +1501,7 @@ export async function validateUpdatePluginProposalActions(
switch (action) {
// if the action is grant plugin update permission
// validate the action
case ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION:
case ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION:
resCauses = await validateGrantUpdatePluginPermissionAction(
actions[index],
pspAddress,
Expand Down Expand Up @@ -1568,7 +1579,7 @@ export async function validateUpdatePluginProposalActions(
switch (action) {
// if the action is grant plugin update permission
// validate the action
case ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION:
case ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION:
resCauses = await validateGrantUpdatePluginPermissionAction(
actions[index],
pspAddress,
Expand Down
7 changes: 3 additions & 4 deletions modules/client/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,10 +484,8 @@ export enum PluginUpdateProposalInValidityCause {
}

export enum DaoUpdateProposalInvalidityCause {
PROPOSAL_NOT_FOUND = "proposalNotFound",
INVALID_ACTIONS = "invalidActions",
INVALID_TO_ADDRESS = "invalidToAddress",
NON_ZERO_CALL_VALUE = "nonZeroCallValue",
INVALID_TO_ADDRESS = "invalidToAddress",
INVALID_UPGRADE_TO_IMPLEMENTATION_ADDRESS =
"invalidUpgradeToImplementationAddress",
INVALID_UPGRADE_TO_AND_CALL_DATA = "invalidUpgradeToAndCallData",
Expand All @@ -498,7 +496,8 @@ export enum DaoUpdateProposalInvalidityCause {

export type DaoUpdateProposalValidity = {
isValid: boolean;
causes: DaoUpdateProposalInvalidityCause[];
actionErrorCauses: DaoUpdateProposalInvalidityCause[];
proposalSettingsErrorCauses: ProposalSettingsErrorCause[];
};

export type DaoUpdateParams = InitializeFromParams & {
Expand Down
2 changes: 1 addition & 1 deletion modules/client/test/integration/client/methods.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2078,7 +2078,7 @@ describe("Client", () => {
TEST_MULTISIG_PROPOSAL_ID,
);
expect(res.isValid).toBe(false);
expect(res.causes).toMatchObject([DaoUpdateProposalInvalidityCause.PROPOSAL_NOT_FOUND]);
expect(res.causes).toMatchObject([ProposalSettingsErrorCause.PROPOSAL_NOT_FOUND]);
});
});
});
Expand Down
41 changes: 28 additions & 13 deletions modules/client/test/unit/client/utils.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1314,7 +1314,7 @@ describe("Test client utils", () => {
{
input: [
ProposalActionTypes.UPGRADE_TO,
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_PLUGIN_UPGRADE_PERMISSION,
],
Expand All @@ -1323,23 +1323,23 @@ describe("Test client utils", () => {
{
input: [
ProposalActionTypes.UPGRADE_TO_AND_CALL,
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_PLUGIN_UPGRADE_PERMISSION,
],
expected: true,
},
{
input: [
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_PLUGIN_UPGRADE_PERMISSION,
],
expected: false,
},
{
input: [
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.UPGRADE_TO,
Expand All @@ -1348,13 +1348,28 @@ describe("Test client utils", () => {
},
{
input: [
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.UPGRADE_TO_AND_CALL,
],
expected: false,
},
{
input: [
ProposalActionTypes.UPGRADE_TO,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_PLUGIN_UPGRADE_PERMISSION,
//
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.GRANT_ROOT_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_ROOT_PERMISSION,
ProposalActionTypes.REVOKE_PLUGIN_UPGRADE_PERMISSION,
],
expected: true,
},
];
for (const { input, expected } of cases) {
const result = containsDaoUpdateAction(input);
Expand All @@ -1369,7 +1384,7 @@ describe("Test client utils", () => {
{ input: [ProposalActionTypes.UPGRADE_TO_AND_CALL], expected: false },
{
input: [
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_PLUGIN_UPGRADE_PERMISSION,
],
Expand All @@ -1385,17 +1400,17 @@ describe("Test client utils", () => {
},
{
input: [
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
],
expected: false,
},
{
input: [
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_PLUGIN_UPGRADE_PERMISSION,
],
Expand All @@ -1415,7 +1430,7 @@ describe("Test client utils", () => {
{ input: [ProposalActionTypes.UPGRADE_TO_AND_CALL], expected: false },
{
input: [
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.GRANT_ROOT_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_ROOT_PERMISSION,
Expand All @@ -1425,7 +1440,7 @@ describe("Test client utils", () => {
},
{
input: [
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.GRANT_ROOT_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_PLUGIN_UPGRADE_PERMISSION,
Expand All @@ -1434,12 +1449,12 @@ describe("Test client utils", () => {
},
{
input: [
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.GRANT_ROOT_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_ROOT_PERMISSION,
ProposalActionTypes.REVOKE_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPDATE_PERMISSION,
ProposalActionTypes.GRANT_PLUGIN_UPGRADE_PERMISSION,
ProposalActionTypes.GRANT_ROOT_PERMISSION,
ProposalActionTypes.APPLY_UPDATE,
ProposalActionTypes.REVOKE_ROOT_PERMISSION,
Expand Down

0 comments on commit a58c9f7

Please sign in to comment.