From a0a2638c4c2cbdf80ebe1a580e8520ce0042f408 Mon Sep 17 00:00:00 2001 From: josemarinas <36479864+josemarinas@users.noreply.github.com> Date: Wed, 7 Sep 2022 15:41:22 +0100 Subject: [PATCH] Fix: Encoding flow when updating plugin settings (#66) * add plugin address param * add addres validation and test * update tests * update changelog * fix test * fix code smell --- javascript/modules/client/CHANGELOG.md | 3 +- javascript/modules/client/README.md | 14 +++ javascript/modules/client/package.json | 2 +- .../modules/client/src/client-addressList.ts | 20 ++- javascript/modules/client/src/client-erc20.ts | 20 ++- .../client/src/internal/interfaces/plugins.ts | 115 +++++++++++------- .../integration/client-addressList.test.ts | 33 ++++- .../test/integration/client-erc20.test.ts | 35 +++++- 8 files changed, 180 insertions(+), 62 deletions(-) diff --git a/javascript/modules/client/CHANGELOG.md b/javascript/modules/client/CHANGELOG.md index c588e3a5f..0ea7a69a2 100644 --- a/javascript/modules/client/CHANGELOG.md +++ b/javascript/modules/client/CHANGELOG.md @@ -17,7 +17,8 @@ TEMPLATE: --> ## [UPCOMING] - +### Fixed +- Encoding flow when updating plugin settings ## 0.4.2-alpha On 2022-09-07 11:19:55 diff --git a/javascript/modules/client/README.md b/javascript/modules/client/README.md index cd990b93b..34ce7265b 100644 --- a/javascript/modules/client/README.md +++ b/javascript/modules/client/README.md @@ -659,7 +659,10 @@ const configActionPrarms: IProposalSettings = { minTurnout: 0.5, // 50% }; +const pluginAddress = "0x1234567890123456789012345678901234567890"; + const configAction = client.encoding.updatePluginSettingsAction( + pluginAddress, configActionPrarms, ); @@ -1180,7 +1183,10 @@ const configActionPrarms: IProposalSettings = { minTurnout: 0.5, // 50% }; +const pluginAddress = "0x1234567890123456789012345678901234567890"; + const configAction = client.encoding.updatePluginSettingsAction( + pluginAddress, configActionPrarms, ); @@ -1580,7 +1586,11 @@ const configActionPrarms: IProposalSettings = { minSupport: 0.3, // 30% minTurnout: 0.5, // 50% }; + +const pluginAddress = "0x1234567890123456789012345678901234567890"; + const configAction = client.encoding.updatePluginSettingsAction( + pluginAddress, configActionPrarms, ); ``` @@ -1609,7 +1619,11 @@ const configActionPrarms: IProposalSettings = { minSupport: 0.3, // 30% minTurnout: 0.5, // 50% }; + +const pluginAddress = "0x1234567890123456789012345678901234567890"; + const configAction = client.encoding.updatePluginSettingsAction( + pluginAddress, configActionPrarms, ); ``` diff --git a/javascript/modules/client/package.json b/javascript/modules/client/package.json index 75a3317d0..ce819b8a8 100644 --- a/javascript/modules/client/package.json +++ b/javascript/modules/client/package.json @@ -1,7 +1,7 @@ { "name": "@aragon/sdk-client", "author": "Aragon Association", - "version": "0.4.2-alpha", + "version": "0.4.3-alpha", "license": "MIT", "main": "dist/index.js", "module": "dist/sdk-client.esm.js", diff --git a/javascript/modules/client/src/client-addressList.ts b/javascript/modules/client/src/client-addressList.ts index d7a0e4ab7..b76cdb730 100644 --- a/javascript/modules/client/src/client-addressList.ts +++ b/javascript/modules/client/src/client-addressList.ts @@ -1,5 +1,4 @@ import { bytesToHex, Random } from "@aragon/sdk-common"; -import { AddressZero } from "@ethersproject/constants"; import { ContextPlugin } from "./context-plugin"; import { ClientCore } from "./internal/core"; import { @@ -36,6 +35,7 @@ import { getDummyAddressListProposalListItem, } from "./internal/temp-mock"; import { getProposalStatus } from "./internal/utils/plugins"; +import { isAddress } from "@ethersproject/address"; // NOTE: This address needs to be set when the plugin has been published and the ID is known const PLUGIN_ID = "0x1234567890123456789012345678901234567890"; @@ -132,12 +132,16 @@ export class ClientAddressList extends ClientCore /** * Computes the parameters to be given when creating a proposal that updates the governance configuration * + * @param {string} pluginAddress * @param {IPluginSettings} params * @return {*} {DaoAction} * @memberof ClientAddressList */ - updatePluginSettingsAction: (params: IPluginSettings): DaoAction => - this._buildUpdatePluginSettingsAction(params), + updatePluginSettingsAction: ( + pluginAddress: string, + params: IPluginSettings, + ): DaoAction => + this._buildUpdatePluginSettingsAction(pluginAddress, params), }; decoding = { /** @@ -348,10 +352,16 @@ export class ClientAddressList extends ClientCore ); } - private _buildUpdatePluginSettingsAction(params: IPluginSettings): DaoAction { + private _buildUpdatePluginSettingsAction( + pluginAddress: string, + params: IPluginSettings, + ): DaoAction { + if (!isAddress(pluginAddress)) { + throw new Error("Invalid plugin address"); + } // TODO: check if to and value are correct return { - to: AddressZero, + to: pluginAddress, value: BigInt(0), data: encodeUpdatePluginSettingsAction(params), }; diff --git a/javascript/modules/client/src/client-erc20.ts b/javascript/modules/client/src/client-erc20.ts index d366666ef..d11896268 100644 --- a/javascript/modules/client/src/client-erc20.ts +++ b/javascript/modules/client/src/client-erc20.ts @@ -38,7 +38,7 @@ import { getDummyErc20Proposal, getDummyErc20ProposalListItem, } from "./internal/temp-mock"; -import { AddressZero } from "@ethersproject/constants"; +import { isAddress } from "@ethersproject/address"; // NOTE: This address needs to be set when the plugin has been published and the ID is known const PLUGIN_ID = "0x1234567890123456789012345678901234567890"; @@ -141,13 +141,17 @@ export class ClientErc20 extends ClientCore implements IClientErc20 { /** * Computes the parameters to be given when creating a proposal that updates the governance configuration * + * @param {string} pluginAddress * @param {IPluginSettings} params * @return {*} {DaoAction} * @memberof ClientErc20 */ // updatePluginSettings() not setConfig() - updatePluginSettingsAction: (params: IPluginSettings): DaoAction => - this._buildUpdatePluginSettingsAction(params), + updatePluginSettingsAction: ( + pluginAddress: string, + params: IPluginSettings, + ): DaoAction => + this._buildUpdatePluginSettingsAction(pluginAddress, params), }; decoding = { /** @@ -322,10 +326,16 @@ export class ClientErc20 extends ClientCore implements IClientErc20 { } //// PRIVATE ACTION BUILDER HANDLERS - private _buildUpdatePluginSettingsAction(params: IPluginSettings): DaoAction { + private _buildUpdatePluginSettingsAction( + pluginAddress: string, + params: IPluginSettings, + ): DaoAction { + if (!isAddress(pluginAddress)) { + throw new Error("Invalid plugin address") + } // TODO: check if to and value are correct return { - to: AddressZero, + to: pluginAddress, value: BigInt(0), data: encodeUpdatePluginSettingsAction(params), }; diff --git a/javascript/modules/client/src/internal/interfaces/plugins.ts b/javascript/modules/client/src/internal/interfaces/plugins.ts index 90b8d133f..93670a41c 100644 --- a/javascript/modules/client/src/internal/interfaces/plugins.ts +++ b/javascript/modules/client/src/internal/interfaces/plugins.ts @@ -16,29 +16,40 @@ import { export interface IClientErc20 extends IClientCore { methods: { createProposal: ( - params: ICreateProposalParams + params: ICreateProposalParams, ) => AsyncGenerator; - voteProposal: (params: IVoteProposalParams) => AsyncGenerator; - executeProposal: (params: IExecuteProposalParams) => AsyncGenerator; + voteProposal: ( + params: IVoteProposalParams, + ) => AsyncGenerator; + executeProposal: ( + params: IExecuteProposalParams, + ) => AsyncGenerator; getMembers: (addressOrEns: string) => Promise; - getProposal: (propoosalId: string) => Promise - getProposals: (params?: IProposalQueryParams) => Promise - getSettings: (pluginAddress: string) => Promise + getProposal: (propoosalId: string) => Promise; + getProposals: ( + params?: IProposalQueryParams, + ) => Promise; + getSettings: (pluginAddress: string) => Promise; }; encoding: { /** Computes the parameters to be given when creating the DAO, so that the plugin is configured */ - updatePluginSettingsAction: (params: IPluginSettings) => DaoAction; + updatePluginSettingsAction: ( + pluginAddress: string, + params: IPluginSettings, + ) => DaoAction; }; decoding: { - updatePluginSettingsAction: (data: Uint8Array) => IPluginSettings - findInterface: (data: Uint8Array) => IInterfaceParams | null + updatePluginSettingsAction: (data: Uint8Array) => IPluginSettings; + findInterface: (data: Uint8Array) => IInterfaceParams | null; }; estimation: { createProposal: ( - params: ICreateProposalParams + params: ICreateProposalParams, ) => Promise; voteProposal: (params: IVoteProposalParams) => Promise; - executeProposal: (params: IExecuteProposalParams) => Promise; + executeProposal: ( + params: IExecuteProposalParams, + ) => Promise; }; } @@ -48,27 +59,40 @@ export interface IClientErc20 extends IClientCore { export interface IClientAddressList extends IClientCore { methods: { createProposal: ( - params: ICreateProposalParams + params: ICreateProposalParams, ) => AsyncGenerator; - voteProposal: (params: IVoteProposalParams) => AsyncGenerator; - executeProposal: (params: IExecuteProposalParams) => AsyncGenerator; + voteProposal: ( + params: IVoteProposalParams, + ) => AsyncGenerator; + executeProposal: ( + params: IExecuteProposalParams, + ) => AsyncGenerator; getMembers: (addressOrEns: string) => Promise; - getProposal: (propoosalId: string) => Promise - getProposals: (params?: IProposalQueryParams) => Promise - getSettings: (pluginAddress: string) => Promise + getProposal: (propoosalId: string) => Promise; + getProposals: ( + params?: IProposalQueryParams, + ) => Promise; + getSettings: (pluginAddress: string) => Promise; }; encoding: { /** Computes the parameters to be given when creating the DAO, so that the plugin is configured */ - updatePluginSettingsAction: (params: IPluginSettings) => DaoAction; + updatePluginSettingsAction: ( + pluginAddress: string, + params: IPluginSettings, + ) => DaoAction; }; decoding: { - updatePluginSettingsAction: (data: Uint8Array) => IPluginSettings - findInterface: (data: Uint8Array) => IInterfaceParams | null + updatePluginSettingsAction: (data: Uint8Array) => IPluginSettings; + findInterface: (data: Uint8Array) => IInterfaceParams | null; }; estimation: { - createProposal: (params: ICreateProposalParams) => Promise; + createProposal: ( + params: ICreateProposalParams, + ) => Promise; voteProposal: (params: IVoteProposalParams) => Promise; - executeProposal: (params: IExecuteProposalParams) => Promise; + executeProposal: ( + params: IExecuteProposalParams, + ) => Promise; }; } @@ -92,7 +116,7 @@ export interface IPluginSettings { } export interface ICreateProposalParams { - pluginAddress: string + pluginAddress: string; metadata: ProposalMetadata; actions?: DaoAction[]; startDate?: Date; @@ -101,12 +125,12 @@ export interface ICreateProposalParams { creatorVote?: VoteValues; } export interface IVoteProposalParams { - pluginAddress: string + pluginAddress: string; vote: VoteValues; proposalId: string; } export interface IExecuteProposalParams { - pluginAddress: string + pluginAddress: string; proposalId: string; } @@ -131,9 +155,9 @@ type NewTokenParams = { }; export type IAddressListPluginInstall = { - addresses: string[], - settings: IPluginSettings -} + addresses: string[]; + settings: IPluginSettings; +}; // STEPS @@ -167,16 +191,15 @@ export type ExecuteProposalStepValue = | { key: ExecuteProposalStep.EXECUTING; txHash: string } | { key: ExecuteProposalStep.DONE }; - // PROPOSAL RETRIEVAL // Long version export type ProposalBase = { - id: string + id: string; dao: { - address: string, - name: string, - }, + address: string; + name: string; + }; creatorAddress: string; metadata: ProposalMetadata; startDate: Date; @@ -184,7 +207,7 @@ export type ProposalBase = { creationDate: Date; actions: Array; status: ProposalStatus; -} +}; export type Erc20Proposal = ProposalBase & { result: Erc20ProposalResult; @@ -192,12 +215,12 @@ export type Erc20Proposal = ProposalBase & { token: Erc20TokenDetails; usedVotingWeight: bigint; votes: Array<{ address: string; vote: VoteValues; weight: bigint }>; -} +}; export type AddressListProposal = ProposalBase & { result: AddressListProposalResult; settings: IProposalSettings; - votes: Array<{ address: string; vote: VoteValues; }>; + votes: Array<{ address: string; vote: VoteValues }>; }; /** @@ -220,7 +243,7 @@ export type ProposalListItemBase = { dao: { address: string; name: string; - }, + }; creatorAddress: string; metadata: ProposalMetadataSummary; startDate: Date; @@ -235,7 +258,7 @@ export type Erc20ProposalListItem = ProposalListItemBase & { export type AddressListProposalListItem = ProposalListItemBase & { result: AddressListProposalResult; -} +}; /** * Contains the human-readable information about a proposal @@ -262,7 +285,7 @@ export enum VoteValues { ABSTAIN = 1, YES = 2, NO = 3, -}; +} export type Erc20ProposalResult = { yes: bigint; @@ -276,17 +299,21 @@ export type AddressListProposalResult = { abstain: number; }; - -type Erc20TokenDetails = { address: string, name: string; symbol: string; decimals: number; }; +type Erc20TokenDetails = { + address: string; + name: string; + symbol: string; + decimals: number; +}; export interface IProposalQueryParams extends IPagination { - sortBy?: ProposalSortBy - addressOrEns?: string + sortBy?: ProposalSortBy; + addressOrEns?: string; } export enum ProposalSortBy { CREATED_AT, NAME, POPULARITY, - VOTES // currently defined as number of proposals + VOTES, // currently defined as number of proposals } diff --git a/javascript/modules/client/test/integration/client-addressList.test.ts b/javascript/modules/client/test/integration/client-addressList.test.ts index 7c85ff196..a32cce779 100644 --- a/javascript/modules/client/test/integration/client-addressList.test.ts +++ b/javascript/modules/client/test/integration/client-addressList.test.ts @@ -371,6 +371,25 @@ describe("Client", () => { expect(installPluginItemItem.data).toBeInstanceOf(Uint8Array); }); + it("Should create an AddressList client and fail to generate a plugin config action with an invalid address", async () => { + const context = new ContextPlugin(contextParamsLocalChain); + const client = new ClientAddressList(context); + + const pluginConfigParams: IPluginSettings = { + minDuration: 100000, + minTurnout: 0.25, + minSupport: 0.51, + }; + + const pluginAddress = "0xinvalid_address"; + expect(() => + client.encoding.updatePluginSettingsAction( + pluginAddress, + pluginConfigParams, + ) + ).toThrow("Invalid plugin address"); + }); + it("Should create an AddressList client and generate a plugin config action action", async () => { const context = new ContextPlugin(contextParamsLocalChain); const client = new ClientAddressList(context); @@ -381,13 +400,17 @@ describe("Client", () => { minSupport: 0.51, }; + const pluginAddress = "0x1234567890123456789012345678901234567890"; + const installPluginItemItem = client.encoding.updatePluginSettingsAction( + pluginAddress, pluginConfigParams, ); expect(typeof installPluginItemItem).toBe("object"); // what does this should be expect(installPluginItemItem.data).toBeInstanceOf(Uint8Array); + expect(installPluginItemItem.to).toBe(pluginAddress); }); }); @@ -400,8 +423,11 @@ describe("Client", () => { minTurnout: 0.5, minSupport: 0.5, }; + + const pluginAddress = "0x1234567890123456789012345678901234567890"; + const updatePluginSettingsAction = client.encoding - .updatePluginSettingsAction(params); + .updatePluginSettingsAction(pluginAddress, params); const decodedParams: IPluginSettings = client.decoding .updatePluginSettingsAction(updatePluginSettingsAction.data); @@ -432,8 +458,11 @@ describe("Client", () => { minTurnout: 0.5, minSupport: 0.5, }; + + const pluginAddress = "0x1234567890123456789012345678901234567890"; + const updatePluginSettingsAction = client.encoding - .updatePluginSettingsAction(params); + .updatePluginSettingsAction(pluginAddress, params); const iface = client.decoding.findInterface( updatePluginSettingsAction.data, ); diff --git a/javascript/modules/client/test/integration/client-erc20.test.ts b/javascript/modules/client/test/integration/client-erc20.test.ts index 3dc0e26c3..8545694c3 100644 --- a/javascript/modules/client/test/integration/client-erc20.test.ts +++ b/javascript/modules/client/test/integration/client-erc20.test.ts @@ -368,6 +368,23 @@ describe("Client", () => { // what does this should be expect(erc20InstallPluginItem.data).toBeInstanceOf(Uint8Array); }); + it("Should encode an update plugin settings action and fail with an invalid address", async () => { + const context = new ContextPlugin(contextParamsLocalChain); + const client = new ClientErc20(context); + const params: IPluginSettings = { + minDuration: 7200, + minTurnout: 0.5, + minSupport: 0.5, + }; + + const pluginAddress = "0xinvalid_address"; + expect(() => + client.encoding.updatePluginSettingsAction( + pluginAddress, + params, + ) + ).toThrow("Invalid plugin address"); + }); it("Should encode an update plugin settings action", async () => { const context = new ContextPlugin(contextParamsLocalChain); const client = new ClientErc20(context); @@ -376,12 +393,16 @@ describe("Client", () => { minTurnout: 0.5, minSupport: 0.5, }; + + const pluginAddress = "0x1234567890123456789012345678901234567890"; + const updatePluginSettingsAction = client.encoding - .updatePluginSettingsAction(params); + .updatePluginSettingsAction(pluginAddress, params); expect(typeof updatePluginSettingsAction).toBe("object"); // what does this should be expect(updatePluginSettingsAction.data).toBeInstanceOf(Uint8Array); + expect(updatePluginSettingsAction.to).toBe(pluginAddress); }); }); @@ -394,8 +415,11 @@ describe("Client", () => { minTurnout: 0.5, minSupport: 0.5, }; + + const pluginAddress = "0x1234567890123456789012345678901234567890"; + const updatePluginSettingsAction = client.encoding - .updatePluginSettingsAction(params); + .updatePluginSettingsAction(pluginAddress, params); const decodedParams: IPluginSettings = client.decoding .updatePluginSettingsAction(updatePluginSettingsAction.data); @@ -426,8 +450,11 @@ describe("Client", () => { minTurnout: 0.5, minSupport: 0.5, }; + + const pluginAddress = "0x1234567890123456789012345678901234567890"; + const updatePluginSettingsAction = client.encoding - .updatePluginSettingsAction(params); + .updatePluginSettingsAction(pluginAddress, params); const iface = client.decoding.findInterface( updatePluginSettingsAction.data, ); @@ -486,7 +513,7 @@ describe("Client", () => { const client = new ClientErc20(context); const pluginAddress: string = - "0x12345678901234567890ยบ1234567890123456789012"; + "0x1234567890123456789012345678901234567890"; const proposals = await client.methods.getSettings(pluginAddress); expect(typeof proposals.minDuration).toBe("number");