Merge pull request #278 from zallo-labs/Z-338-fix-policy-activation

Z 338 fix policy activation

Author: hbriese

api/dbschema/edgeql-js/__spec__.ts

@@ -479,6 +479,7 @@ export type $PolicyλShape = $.typeutil.flatten<$PolicyStateλShape & {
   "delay": $.PropertyDesc<$uint32, $.Cardinality.One, false, false, false, true>;
   "name": $.PropertyDesc<$BoundedStr, $.Cardinality.One, false, false, false, false>;
   "threshold": $.PropertyDesc<$uint16, $.Cardinality.One, false, false, false, false>;
+  "hash": $.PropertyDesc<$Bytes32, $.Cardinality.One, false, false, false, false>;
   "<policies[is Account]": $.LinkDesc<$Account, $.Cardinality.Many, {}, false, false,  false, false>;
   "<policy[is Proposal]": $.LinkDesc<$Proposal, $.Cardinality.Many, {}, false, false,  false, false>;
   "<policy[is Transaction]": $.LinkDesc<$Transaction, $.Cardinality.Many, {}, false, false,  false, false>;

api/dbschema/edgeql-js/modules/default.ts

@@ -228,6 +228,7 @@ export namespace $default {
     "delay": number;
     "name": string;
     "threshold": number;
+    "hash": string;
   }
   export interface Rejection extends ProposalResponse {}
   export interface RemovedPolicy extends PolicyState {}

api/dbschema/interfaces.ts

@@ -0,0 +1,10 @@
+CREATE MIGRATION m17arqyctfz6btivr4soubbql7ruianngjqd3y4p5sdnbqcgvcz4tq
+    ONTO m1ob6arfwyamzcpgk2hgzhby3iwv3moewthqacorsw2mtyjkopdoaq
+{
+  ALTER TYPE default::Policy {
+      CREATE REQUIRED PROPERTY hash: default::Bytes32 {
+          SET REQUIRED USING (<default::Bytes32>'0x0000000000000000000000000000000000000000000000000000000000000000');
+      };
+      CREATE INDEX ON ((.account, .key, .hash));
+  };
+};

api/dbschema/migrations/00003-m17arqy.edgeql

@@ -13,7 +13,7 @@ module default {
       rewrite insert using (
         .isLatest if (__specified__.isLatest) else
         # ((.activationBlock ?? 0n) > (latestPolcy(.account, .key)).activationBlock ?? -1n)
-        ((.activationBlock ?? 0n) > (assert_single((select Policy filter .account = __subject__.account and .key = __subject__.key and .isLatest)).activationBlock ?? -1n)) 
+        ((.activationBlock ?? 0n) > (assert_single((select Policy filter .account = __subject__.account and .key = __subject__.key and .isLatest)).activationBlock ?? -1n))
       )
     }
     required initState := .activationBlock ?= 0;
@@ -39,6 +39,7 @@ module default {
   }
 
   type Policy extending PolicyState {
+    required hash: Bytes32;
     required name: BoundedStr;
     required threshold: uint16;
     multi approvers: Approver;
@@ -47,6 +48,8 @@ module default {
     required allowMessages: bool { default := false; }
     required delay: uint32 { default := 0; }
 
+    index on ((.account, .key, .hash));
+
     trigger update_proposals_when_latest after insert, update for each
     when (__new__.isLatest) do (
       update Proposal filter .account = __new__.account and .policy.key = __new__.key and

api/dbschema/policy.esdl

@@ -389,6 +389,7 @@ type Policy implements Node & PolicyState {
   delay: Float!
   draft: PolicyState
   hasBeenActive: Boolean!
+  hash: Bytes32!
   id: ID!
   initState: Boolean!
   isActive: Boolean!

api/schema.graphql

@@ -1,7 +1,10 @@
 with account := (select Account filter .address = <UAddress>$account),
-     proposal := (select SystemTx filter .hash = <Bytes32>$systxHash).proposal,
      key := <uint16>$key,
-     new := assert_single((select PolicyState filter .account = account and .key = key and (.proposal ?= proposal or .initState))),
+     new := assert_single((
+       select PolicyState filter .account = account and .key = key and 
+         ([is Policy].hash ?= <optional Bytes32>$hash or PolicyState is RemovedPolicy) and
+         (not exists .activationBlock or .activationBlock ?= 0)
+     )),
      old := assert_single((select PolicyState filter .account = account and key = .key and .isLatest and .id != new.id)),
      activationBlock := <bigint>$activationBlock,
      isLater := (activationBlock > (old.activationBlock ?? -1n)),

api/src/feat/policies/activate-policy.edgeql

@@ -4,8 +4,8 @@ import type {Executor} from "edgedb";
 
 export type ActivatePolicyArgs = {
   readonly "account": string;
-  readonly "systxHash": string;
   readonly "key": number;
+  readonly "hash"?: string | null;
   readonly "activationBlock": bigint;
 };
 
@@ -18,9 +18,12 @@ export type ActivatePolicyReturns = {
 export function activatePolicy(client: Executor, args: ActivatePolicyArgs): Promise<ActivatePolicyReturns> {
   return client.queryRequiredSingle(`\
 with account := (select Account filter .address = <UAddress>$account),
-     proposal := (select SystemTx filter .hash = <Bytes32>$systxHash).proposal,
      key := <uint16>$key,
-     new := assert_single((select PolicyState filter .account = account and .key = key and (.proposal ?= proposal or .initState))),
+     new := assert_single((
+       select PolicyState filter .account = account and .key = key and 
+         ([is Policy].hash ?= <optional Bytes32>$hash or PolicyState is RemovedPolicy) and
+         (not exists .activationBlock or .activationBlock ?= 0)
+     )),
      old := assert_single((select PolicyState filter .account = account and key = .key and .isLatest and .id != new.id)),
      activationBlock := <bigint>$activationBlock,
      isLater := (activationBlock > (old.activationBlock ?? -1n)),

api/src/feat/policies/activate-policy.query.ts

@@ -3,6 +3,7 @@ with account := (select Account filter .address = <UAddress>$account),
      tx := ((select Transaction filter .id = txId) if exists txId else {})
 for p in array_unpack(<array<json>>$policies) union (
   with policy := (insert Policy {
+    hash := <Bytes32>p['hash'],
     account := account,
     key := <uint16>p['key'],
     proposal := tx,

api/src/feat/policies/insert-policies.edgeql

@@ -20,6 +20,7 @@ with account := (select Account filter .address = <UAddress>$account),
      tx := ((select Transaction filter .id = txId) if exists txId else {})
 for p in array_unpack(<array<json>>$policies) union (
   with policy := (insert Policy {
+    hash := <Bytes32>p['hash'],
     account := account,
     key := <uint16>p['key'],
     proposal := tx,

api/src/feat/policies/insert-policies.query.ts

@@ -1,6 +1,6 @@
 import { Injectable } from '@nestjs/common';
 import { EventsWorker, EventData, Log } from '../events/events.worker';
-import { ACCOUNT_ABI, PolicyKey, asPolicyKey, asUAddress, asUUID } from 'lib';
+import { ACCOUNT_ABI, Hex, PolicyKey, asPolicyKey, asUAddress, asUUID } from 'lib';
 import { Chain } from 'chains';
 import { DatabaseService } from '~/core/database';
 import { getAbiItem } from 'viem';
@@ -29,6 +29,7 @@ export class PoliciesEventsProcessor {
       chain,
       log,
       asPolicyKey(log.args.key),
+      log.args.hash,
     );
 
     if (policyId)
@@ -46,20 +47,21 @@ export class PoliciesEventsProcessor {
       this.policies.event({ event: PolicyEvent.removed, account, policyId: asUUID(policyId) });
   }
 
-  private async markStateAsActive(chain: Chain, log: Log, key: PolicyKey) {
-    // FIXME: when multiple policies are activated in one block, the wrong one may be marked as active
-    // This *always* occurs when a policy is activated by a policy update transaction
-
+  private async markStateAsActive(chain: Chain, log: Log, key: PolicyKey, hash?: Hex) {
     const account = asUAddress(log.address, chain);
     const r = await this.db.exec(activatePolicy, {
       account,
       key,
-      systxHash: log.transactionHash,
+      hash,
       activationBlock: log.blockNumber,
     });
 
     await Promise.all(r.pendingTransactions.map((id) => this.transactions.tryExecute(asUUID(id))));
 
+    if (!r.new) {
+      // TODO: this shouldn't happen
+    }
+
     return { account, old: r.old, new: r.new };
   }
 }

api/src/feat/policies/policies.events.ts

@@ -3,7 +3,7 @@ import { GraphQLBigInt } from 'graphql-scalars';
 import { Account } from '../accounts/accounts.model';
 import { Transaction } from '../transactions/transactions.model';
 import * as eql from '~/edgeql-interfaces';
-import { Address, PolicyKey, Selector, UAddress } from 'lib';
+import { Address, Hex, PolicyKey, Selector, UAddress } from 'lib';
 import { Approver } from '../approvers/approvers.model';
 import {
   CustomNodeType,
@@ -22,6 +22,7 @@ import {
   AbiFunctionField,
   PolicyKeyField,
   UAddressField,
+  Bytes32Field,
 } from '~/common/scalars';
 
 @NodeType()
@@ -113,6 +114,9 @@ export class PolicyState extends Node {
 
 @NodeType({ implements: [PolicyState] })
 export class Policy extends PolicyState {
+  @Bytes32Field()
+  hash: Hex;
+
   @Field(() => String)
   name: string;
 

api/src/feat/policies/policies.model.ts

@@ -13,6 +13,7 @@ import {
   UUID,
   asUUID,
   PolicyKey,
+  hashPolicy,
 } from 'lib';
 import { TransactionsService } from '../transactions/transactions.service';
 import {
@@ -128,9 +129,10 @@ export class PoliciesService {
         return {
           policy,
           state: {
-            name: 'Policy ' + input.key,
             ...policyState,
+            name: input.name || 'Policy ' + input.key,
           },
+          hash: hashPolicy(policy),
         };
       })
       .filter(Boolean);
@@ -157,9 +159,10 @@ export class PoliciesService {
       await this.db.exec(insertPolicies, {
         account,
         transaction,
-        policies: changedPolicies.map(({ state }) => ({
-          ...(isInitialization && { activationBlock: 0n }),
+        policies: changedPolicies.map(({ state, hash }) => ({
+          hash,
           ...state,
+          ...(isInitialization && { activationBlock: 0n }),
         })),
       })
     ).map((p) => ({ ...p, id: asUUID(p.id), key: asPolicyKey(p.key) }));

api/src/feat/policies/policies.service.ts

@@ -22,6 +22,7 @@ import Decimal from 'decimal.js';
 import { PoliciesService } from '../policies/policies.service';
 import { PricesService } from '~/feat/prices/prices.service';
 import { TokensService } from '~/feat/tokens/tokens.service';
+import { zeroHash } from 'viem';
 
 const signature = '0x1234' as Hex;
 
@@ -124,6 +125,7 @@ describe(TransactionsService.name, () => {
       await e
         .insert(e.Policy, {
           account: selectAccount(accountId),
+          hash: zeroHash,
           key: 0,
           name: 'Policy 0',
           threshold: 0,
@@ -438,6 +440,7 @@ describe(TransactionsService.name, () => {
         const policy = await db.query(
           e.insert(e.Policy, {
             account: selectAccount(user1Account1),
+            hash: zeroHash,
             key: 1,
             name: 'Policy 1',
             proposal: selectTransaction(id),