fixes and enhancements to DaoCreator.setSchemes (#314)

daostack · Aug 6, 2018 · cc2a678 · cc2a678
1 parent 9dc31a1
commit cc2a678
Show file tree

Hide file tree

Showing 15 changed files with 212 additions and 34 deletions.
diff --git a/lib/contractWrapperBase.ts b/lib/contractWrapperBase.ts
@@ -131,18 +131,28 @@ export abstract class ContractWrapperBase implements IContractWrapperBase {
    *
    * @param {any} params -- parameters as the contract.setParameters function expects them.
    */
-  public async setParameters(...params: Array<any>): Promise<ArcTransactionDataResult<Hash>> {
+  public setParameters(...params: Array<any>): Promise<ArcTransactionDataResult<Hash>> {
     throw new Error("setParameters has not been not implemented by the contract wrapper");
   }
 
   /**
    * Given a hash, returns the associated parameters as an object.
    * @param paramsHash
    */
-  public async getParameters(paramsHash: Hash): Promise<any> {
+  public getParameters(paramsHash: Hash): Promise<any> {
     throw new Error("getParameters has not been not implemented by the contract wrapper");
   }
 
+  /**
+   * Given an object containing the contract's parameters, return the hash
+   * that would be used to represent them in Arc.  Note this doesn't indicate
+   * whether the parameters have been registered with the contract.
+   * @param params
+   */
+  public getParametersHash(params: any): Promise<Hash> {
+    throw new Error("getParametersHash has not been not implemented by the contract wrapper");
+  }
+
   /**
    * Given an avatar address, returns the schemes parameters hash
    * @param avatarAddress
@@ -157,15 +167,15 @@ export abstract class ContractWrapperBase implements IContractWrapperBase {
    * in which the parameters appear in the contract's Parameters struct.
    * @param paramsHash
    */
-  public async getParametersArray(paramsHash: Hash): Promise<Array<any>> {
+  public getParametersArray(paramsHash: Hash): Promise<Array<any>> {
     return this.contract.parameters(paramsHash);
   }
 
   /**
    * Returns the controller associated with the given avatar
    * @param avatarAddress
    */
-  public async getController(avatarAddress: Address): Promise<any> {
+  public getController(avatarAddress: Address): Promise<any> {
     const controllerService = new ControllerService(avatarAddress);
     return controllerService.getController();
   }
@@ -247,6 +257,10 @@ export abstract class ContractWrapperBase implements IContractWrapperBase {
     return this.getParameters(paramsHash);
   }
 
+  protected _getParametersHash(...params: Array<any>): Promise<Hash> {
+    return this.contract.getParametersHash(...params);
+  }
+
   /**
    * See [Web3EventService.createEventFetcherFactory](Web3EventService#createEventFetcherFactory).
    *

diff --git a/lib/iContractWrapperBase.ts b/lib/iContractWrapperBase.ts
@@ -17,6 +17,7 @@ export interface IContractWrapperBase extends HasContract {
   setParameters(...params: Array<any>): Promise<ArcTransactionDataResult<Hash>>;
   getParameters(paramsHash: Hash): Promise<any>;
   getSchemeParametersHash(avatarAddress: Address): Promise<Hash>;
+  getParametersHash(params: any): Promise<Hash>;
   getParametersArray(paramsHash: Hash): Promise<Array<any>>;
   getController(avatarAddress: Address): Promise<any>;
 }

diff --git a/lib/test/wrappers/testWrapper.ts b/lib/test/wrappers/testWrapper.ts
@@ -20,7 +20,7 @@ export class TestWrapperWrapper extends ContractWrapperBase {
     return "abc";
   }
 
-  public async setParameters(params: AbsoluteVoteParams): Promise<ArcTransactionDataResult<Hash>> {
+  public setParameters(params: AbsoluteVoteParams): Promise<ArcTransactionDataResult<Hash>> {
     params = Object.assign({},
       {
         ownerVote: true,

diff --git a/lib/wrappers/absoluteVote.ts b/lib/wrappers/absoluteVote.ts
@@ -61,7 +61,21 @@ export class AbsoluteVoteWrapper extends IntVoteInterfaceWrapper {
     });
   }
 
-  public async setParameters(params: AbsoluteVoteParams): Promise<ArcTransactionDataResult<Hash>> {
+  public getParametersHash(params: AbsoluteVoteParams): Promise<Hash> {
+    params = Object.assign({},
+      {
+        ownerVote: true,
+        votePerc: 50,
+      },
+      params);
+
+    return this._getParametersHash(
+      params.reputation,
+      params.votePerc,
+      params.ownerVote);
+  }
+
+  public setParameters(params: AbsoluteVoteParams): Promise<ArcTransactionDataResult<Hash>> {
 
     params = Object.assign({},
       {

diff --git a/lib/wrappers/contributionReward.ts b/lib/wrappers/contributionReward.ts
@@ -438,7 +438,21 @@ export class ContributionRewardWrapper extends ProposalGeneratorBase implements
     return rewardsArray;
   }
 
-  public async setParameters(params: ContributionRewardParams): Promise<ArcTransactionDataResult<Hash>> {
+  public getParametersHash(params: ContributionRewardParams): Promise<Hash> {
+    params = Object.assign({},
+      {
+        orgNativeTokenFee: 0,
+      },
+      params);
+
+    return this._getParametersHash(
+      params.orgNativeTokenFee,
+      params.voteParametersHash,
+      params.votingMachineAddress
+    );
+  }
+
+  public setParameters(params: ContributionRewardParams): Promise<ArcTransactionDataResult<Hash>> {
 
     this.validateStandardSchemeParams(params);
 

diff --git a/lib/wrappers/daoCreator.ts b/lib/wrappers/daoCreator.ts
@@ -2,7 +2,7 @@
 import * as BigNumber from "bignumber.js";
 import { computeForgeOrgGasLimit } from "../../gasLimits.js";
 import { AvatarService } from "../avatarService";
-import { Address, SchemePermissions } from "../commonTypes";
+import { Address, Hash, SchemePermissions } from "../commonTypes";
 import { ConfigService } from "../configService";
 import { ContractWrapperBase } from "../contractWrapperBase";
 import { ContractWrapperFactory } from "../contractWrapperFactory";
@@ -137,6 +137,20 @@ export class DaoCreatorWrapper extends ContractWrapperBase {
      */
     const eventContext = TransactionService.newTxEventContext(functionName, payload, options);
 
+    const votingMachineParamsHashes = new Map<Address, Set<Hash>>();
+    const paramsHashCacheSet = (address: Address, hash: Hash): void => {
+      let hashes = votingMachineParamsHashes.get(address);
+      if (!hashes) {
+        hashes = new Set<Hash>();
+      }
+      hashes.add(hash);
+    };
+
+    const paramsHashCacheHasHash = (address: Address, hash: Hash): boolean => {
+      const hashes = votingMachineParamsHashes.get(address);
+      return hashes && hashes.has(hash);
+    };
+
     const avatarService = new AvatarService(options.avatar);
     const reputationAddress = await avatarService.getNativeReputationAddress();
     const configuredVotingMachineName = ConfigService.get("defaultVotingMachine");
@@ -162,6 +176,10 @@ export class DaoCreatorWrapper extends ContractWrapperBase {
       Object.assign(defaultVotingMachineParams, { txEventContext: eventContext }));
 
     const defaultVoteParametersHash = txResult.result;
+    paramsHashCacheSet(defaultVotingMachine.address, defaultVoteParametersHash);
+
+    // avoid nonce collisions
+    await txResult.watchForTxMined();
 
     const initialSchemesSchemes = [];
     const initialSchemesParams = [];
@@ -222,33 +240,47 @@ export class DaoCreatorWrapper extends ContractWrapperBase {
           schemeVotingMachineParams.votingMachineAddress = schemeVotingMachine.address;
         }
 
-        schemeVotingMachineParams = Object.assign(defaultVotingMachineParams, schemeVotingMachineParams);
+        schemeVotingMachineParams = Object.assign({}, defaultVotingMachineParams, schemeVotingMachineParams);
         /**
          * get the voting machine parameters
          */
-        txResult = await schemeVotingMachine.setParameters(schemeVotingMachineParams);
-        schemeVoteParametersHash = txResult.result;
+        schemeVoteParametersHash = await schemeVotingMachine.getParametersHash(schemeVotingMachineParams);
+
+        // avoid nonce collisions and avoid unnecessary transactions
+        if (!paramsHashCacheHasHash(schemeVotingMachine.address, schemeVoteParametersHash)) {
+          txResult = await schemeVotingMachine.setParameters(schemeVotingMachineParams);
+          // avoid nonce collisions
+          await txResult.watchForTxMined();
+          paramsHashCacheSet(schemeVotingMachine.address, schemeVoteParametersHash);
+        }
       } else {
         // using the defaults
-        schemeVotingMachineParams = Object.assign(defaultVotingMachineParams, { txEventContext: eventContext });
+        schemeVotingMachineParams = Object.assign({}, defaultVotingMachineParams, { txEventContext: eventContext });
         schemeVoteParametersHash = defaultVoteParametersHash;
       }
 
-      /**
-       * This is the set of all possible parameters from which the current scheme
-       * will choose just the ones it requires
-       */
-      txResult = await scheme.setParameters(
-        Object.assign(
-          {
-            txEventContext: eventContext,
-            voteParametersHash: schemeVoteParametersHash,
-            votingMachineAddress: schemeVotingMachineParams.votingMachineAddress,
-          },
-          schemeOptions
-        ));
-
-      const schemeParamsHash = txResult.result;
+      const schemeParameters = Object.assign(
+        {
+          txEventContext: eventContext,
+          voteParametersHash: schemeVoteParametersHash,
+          votingMachineAddress: schemeVotingMachineParams.votingMachineAddress,
+        },
+        schemeOptions
+      );
+
+      const schemeParamsHash = await scheme.getParametersHash(schemeParameters);
+
+      // avoid nonce collisions and avoid unnecessary transactions
+      if (!paramsHashCacheHasHash(scheme.address, schemeParamsHash)) {
+        /**
+         * This is the set of all possible parameters from which the current scheme
+         * will choose just the ones it requires
+         */
+        txResult = await scheme.setParameters(schemeParameters);
+        // avoid nonce collisions
+        await txResult.watchForTxMined();
+        paramsHashCacheSet(scheme.address, schemeParamsHash);
+      }
 
       initialSchemesSchemes.push(scheme.address);
       initialSchemesParams.push(schemeParamsHash);

diff --git a/lib/wrappers/genesisProtocol.ts b/lib/wrappers/genesisProtocol.ts
@@ -841,6 +841,31 @@ export class GenesisProtocolWrapper extends IntVoteInterfaceWrapper implements S
     return this.convertProposalPropsArrayToObject(proposalParams, proposalId);
   }
 
+  public async getParametersHash(params: GenesisProtocolParams): Promise<Hash> {
+
+    params = Object.assign({},
+      await GetDefaultGenesisProtocolParameters(),
+      params);
+
+    return this._getParametersHash(
+      [
+        params.preBoostedVoteRequiredPercentage || 0,
+        params.preBoostedVotePeriodLimit,
+        params.boostedVotePeriodLimit,
+        params.thresholdConstA || 0,
+        params.thresholdConstB || 0,
+        params.minimumStakingFee || 0,
+        params.quietEndingPeriod,
+        params.proposingRepRewardConstA || 0,
+        params.proposingRepRewardConstB || 0,
+        params.stakerFeeRatioForVoters || 0,
+        params.votersReputationLossRatio || 0,
+        params.votersGainRepRatioFromLostRep || 0,
+        params.daoBountyConst || 0,
+        params.daoBountyLimit || 0,
+      ]);
+  }
+
   /**
    * Set the contract parameters.
    * @param {GenesisProtocolParams} params

diff --git a/lib/wrappers/globalConstraintRegistrar.ts b/lib/wrappers/globalConstraintRegistrar.ts
@@ -168,7 +168,14 @@ export class GlobalConstraintRegistrarWrapper extends ProposalGeneratorBase impl
     return this.convertProposalPropsArrayToObject(proposalParams, proposalId);
   }
 
-  public async setParameters(params: StandardSchemeParams): Promise<ArcTransactionDataResult<Hash>> {
+  public getParametersHash(params: StandardSchemeParams): Promise<Hash> {
+    return this._getParametersHash(
+      params.voteParametersHash,
+      params.votingMachineAddress
+    );
+  }
+
+  public setParameters(params: StandardSchemeParams): Promise<ArcTransactionDataResult<Hash>> {
 
     this.validateStandardSchemeParams(params);
 

diff --git a/lib/wrappers/schemeRegistrar.ts b/lib/wrappers/schemeRegistrar.ts
@@ -127,7 +127,15 @@ export class SchemeRegistrarWrapper extends ProposalGeneratorBase implements Sch
     return new ArcTransactionProposalResult(txResult.tx, this.contract, await this.getVotingMachine(options.avatar));
   }
 
-  public async setParameters(params: SchemeRegistrarParams): Promise<ArcTransactionDataResult<Hash>> {
+  public getParametersHash(params: SchemeRegistrarParams): Promise<Hash> {
+    return this._getParametersHash(
+      params.voteParametersHash,
+      params.voteRemoveParametersHash ? params.voteRemoveParametersHash : params.voteParametersHash,
+      params.votingMachineAddress
+    );
+  }
+
+  public setParameters(params: SchemeRegistrarParams): Promise<ArcTransactionDataResult<Hash>> {
 
     this.validateStandardSchemeParams(params);
 

diff --git a/lib/wrappers/tokenCapGC.ts b/lib/wrappers/tokenCapGC.ts
@@ -13,7 +13,13 @@ export class TokenCapGCWrapper extends ContractWrapperBase {
   public friendlyName: string = "Token Cap Global Constraint";
   public factory: IContractWrapperFactory<TokenCapGCWrapper> = TokenCapGCFactory;
 
-  public async setParameters(params: TokenCapGcParams): Promise<ArcTransactionDataResult<Hash>> {
+  public getParametersHash(params: TokenCapGcParams): Promise<Hash> {
+    return this._getParametersHash(
+      params.token,
+      params.cap || 0
+    );
+  }
+  public setParameters(params: TokenCapGcParams): Promise<ArcTransactionDataResult<Hash>> {
 
     if (!params.token) {
       throw new Error("token must be set");

diff --git a/lib/wrappers/upgradeScheme.ts b/lib/wrappers/upgradeScheme.ts
@@ -94,7 +94,14 @@ export class UpgradeSchemeWrapper extends ProposalGeneratorBase implements Schem
     return new ArcTransactionProposalResult(txResult.tx, this.contract, await this.getVotingMachine(options.avatar));
   }
 
-  public async setParameters(params: StandardSchemeParams): Promise<ArcTransactionDataResult<Hash>> {
+  public getParametersHash(params: StandardSchemeParams): Promise<Hash> {
+    return this._getParametersHash(
+      params.voteParametersHash,
+      params.votingMachineAddress
+    );
+  }
+
+  public setParameters(params: StandardSchemeParams): Promise<ArcTransactionDataResult<Hash>> {
 
     this.validateStandardSchemeParams(params);
 

diff --git a/lib/wrappers/vestingScheme.ts b/lib/wrappers/vestingScheme.ts
@@ -300,7 +300,14 @@ export class VestingSchemeWrapper extends ProposalGeneratorBase implements Schem
     return agreement;
   }
 
-  public async setParameters(params: StandardSchemeParams): Promise<ArcTransactionDataResult<Hash>> {
+  public getParametersHash(params: StandardSchemeParams): Promise<Hash> {
+    return this._getParametersHash(
+      params.voteParametersHash,
+      params.votingMachineAddress
+    );
+  }
+
+  public setParameters(params: StandardSchemeParams): Promise<ArcTransactionDataResult<Hash>> {
 
     this.validateStandardSchemeParams(params);
 

diff --git a/lib/wrappers/voteInOrganizationScheme.ts b/lib/wrappers/voteInOrganizationScheme.ts
@@ -118,7 +118,14 @@ export class VoteInOrganizationSchemeWrapper extends ProposalGeneratorBase imple
     return this.convertProposalPropsArrayToObject(proposalParams, proposalId);
   }
 
-  public async setParameters(params: StandardSchemeParams): Promise<ArcTransactionDataResult<Hash>> {
+  public getParametersHash(params: StandardSchemeParams): Promise<Hash> {
+    return this._getParametersHash(
+      params.voteParametersHash,
+      params.votingMachineAddress
+    );
+  }
+
+  public setParameters(params: StandardSchemeParams): Promise<ArcTransactionDataResult<Hash>> {
 
     this.validateStandardSchemeParams(params);
 

diff --git a/test/absoluteVote.ts b/test/absoluteVote.ts
@@ -0,0 +1,25 @@
+import { assert } from "chai";
+import {
+  AbsoluteVoteFactory,
+} from "../lib/wrappers/absoluteVote";
+import * as helpers from "./helpers";
+
+describe("AbsoluteVote", () => {
+
+  it("can get params hash", async () => {
+
+    const absoluteVote = await AbsoluteVoteFactory.new();
+
+    const params = await {
+      ownerVote: true,
+      reputation: helpers.SOME_ADDRESS,
+      votePerc: 50,
+    };
+
+    const paramsHashSet = (await absoluteVote.setParameters(params)).result;
+
+    const paramsHashGet = await absoluteVote.getParametersHash(params);
+
+    assert.equal(paramsHashGet, paramsHashSet, "Hashes are not the same");
+  });
+});