Add mint tokens motion support #16

willm30 · 2023-02-10T20:48:45Z

Description

This pr adds block-ingestor support for the MotionCreated event, specifically when it wraps the mint tokens action.

I have coupled the MotionCreated event listeners with the Voting Reputation extension, such that as soon as it is initialised, or if trackExtensions determines that it is initialised on block-ingestor start up, it will also listen for MotionCreated events.

Testing

To test, you will need to:

Be on this cdapp branch: Port root motion saga colonyCDapp#261. Make sure the block ingestor is commented out in in colony-cdapp-env-orchestration.yaml:

  # block-ingestor:
  #   container_name: 'ingestor'
  #   image: colony-cdapp-dev-env/block-ingestor:latest
  #   volumes:
  #     - '../amplify/mock-data:/colonyCDapp/amplify/mock-data'
  #   ports:
  #     - '10001:10001'
  #   depends_on:
  #     network-contracts:
  #       condition: service_healthy
  #     amplify:
  #       condition: service_healthy
  #   links:
  #     - "network-contracts:network-contracts.docker"
  #     - "amplify:amplify.docker"

Add reputation to the user. This can be done by:

Copying the following into a new tab and ensuring it says the reputation miner is on: http://localhost:3001/reputation/monitor/toggle
In a new terminal window, run npm run truffle console
Paste the following:
- c = await IColony.at('{colonyAddress}') (where colony address is the address of the colony you're testing out)
  Then
- c.emitDomainReputationReward(1, accounts[0], '1000000000000000000') (make sure you're logged in with dev wallet 1).
- Wait for the reputation mining cycle to complete and you should see the reputation update in the ui.

Install and initialise the Voting Reputation (Governance) extension
Click the "Test Mint Tokens" button. If all is set up correctly, you should be redirected to a motions screen.

Testing should reveal that only one motion is created in the db, and it should be created in the colony you clicked "test mint tokens" in.

You should also terminate the block-ingestor and restart it, and you should see a listener attached to the MotionCreated event as a result of trackExtensions determining that the Voting Reputation Extn is installed.

Notes

In my dev env, I've been noticing that the event listener attached to the MotionCreated event is firing for each colony, as well as often multiple times for the same colony.

I have guarded against the motion being associated with the wrong colony in the db, but you may see errors in the block ingestor terminal when creating a motion in the db, because it may attempt to create the same motion twice. This should not be a problem and should not actually create more than one motion per tx hash. You can verify this in the ui as well as at localhost:20002 by ensuring only one motion is created per click.

I have also noticed that sometimes when you click the button, it redirects but the block ingestor doesn't pick up the event and so the action page doesn't load. If this happens, go back to Colony Home and refresh & try again. We suspect this may be the result of hot reloading the block ingestor and therefore only a problem in dev.

Finally, I've noticed that a couple of TokensMinted events get picked up straight after, with an amount of undefined and 0. I've added logic to guard against making a db mutatation when this is the case.

Troubleshooting

If you are experiencing an incorrent nonce error at colony create when running the block ingestor separately, it may be caused by the await provider.getNetwork() call on line 45 of index.ts. This is just a theory but commenting it out and the getChainId() call on the line below solved it for me the first time I tried it.

willm30 · 2023-02-13T19:21:15Z

src/eventQueue.ts

+    this.queue.push(event);
+    verbose(
+      'Event added to the queue:',
+      event?.signature || ContractEventsSignatures.UnknownEvent,


Changes in this file are just from prettier & correcting the "Uknown" event typo

willm30 · 2023-02-13T19:25:23Z

src/utils/eventListeners.ts

+ * It basically does away with all the boilerplate of setting up topics, setting
+ * the event listener, parsing the received event
+ */
+export const eventListenerGenerator = async (


Have moved all the listeners to their own file to separate them from the helpers they use, especially now I've written a few for eventListenerGenerator

willm30 · 2023-02-13T19:26:03Z

src/utils/eventListeners.ts

+ * Voting Reputation Client specific event listener,
+ * which uses `eventListenerGenerator` under the hood
+ */
+export const addMotionEventListener = async (


This is the only addition, aside from refactoring event listener generator

willm30 · 2023-02-13T19:28:18Z

src/utils/events.ts

 */
-export const eventListenerGenerator = async (
-  eventSignature: ContractEventsSignatures,
+export const getClientAndProviderForELG = async (


Written a few helper functions for the event listener generator, since the exact logic seems to change depending on the client type. E.g. which client / provider to use, which filter to use, which contract address to use in the event, when to add the event to the queue...

I expect splitting up the various parts of adding an event listener will make it easier to extend as we continue porting over functionality.

willm30 · 2023-02-13T19:31:03Z

src/utils/events.ts

+         * In the case of Voting Rep, only add the event to the queue if the colony address
+         * associated with the event is the same as the colony address the listener is associated with.
+         */
+        const { colonyId } = await query('getColonyExtension', {


Using this to get the colony the voting reputation extension is installed in. This is a guard against the motion being written to the db with the wrong colony (which is possible when the behaviour I'm seeing in development occurs, where the event fires on all colonies)

willm30 · 2023-02-13T19:32:58Z

src/utils/motions.ts

+import { verbose } from './logger';
+import { getColonyTokenAddress, getColonyTokenData } from './tokens';
+
+export const getParsedActionFromMotion = async (


This is how to get the underlying action associated with the motion, e.g mint tokens

willm30 · 2023-02-13T19:33:25Z

src/utils/motions.ts

+  const amount = actionArgs[0].toString();
+  const tokenAddress = await getColonyTokenAddress(colonyAddress);
+  const tokenData = await getColonyTokenData(tokenAddress, colonyClient);
+  await mutate('createColonyAction', {


This adds all the data the ui needs for the mint token motion

willm30 · 2023-02-14T11:46:30Z

src/eventProcessor.ts

-        'tokens were minted in colony:',
-        colonyAddress,
-      );
+      if (amount && amount.toString() !== '0') {


Added this because the TokensMinted event is picked up after the MotionCreated event, twice actually. The amounts are undefined and 0, so guarding against that here.

Would be interested to know why this happens @rdig?

Is the motion set to very short timeout ? Ie: it finishes instantly ? Otherwise I can't really explain this.

The underlying action / event doesn't get triggered until the motion finishes (and that is done manually).

If that isn't the case I don't really understand what's going on. Maybe ask @area for his input ?

jakubcolony

I got it to work in conjunction with CDapp#263, but I'm only commenting for now - I had a chat with Raul today and I'll need to refactor the way we handle events that comprise an action and that will probably require some changes to this PR subsequently

jakubcolony · 2023-02-14T12:37:43Z

src/domains.ts

+export const formatDomain = (colonyAddress: string, domainId: string) =>
+  `${colonyAddress}_${domainId.toString()}`;


That could live in the utils folder

jakubcolony · 2023-02-14T13:13:58Z

src/trackExtensions.ts

+    if (Extension.VotingReputation === extensionId) {
+      addMotionEventListener(ContractEventsSignatures.MotionCreated, colony);
+    }


In eventProcessor, this listener is added on ExtensionInitialised event, and here you add it for all installed (but not necessarily initialised) extensions. Is that intentional? If not, you could move it a few lines below, there's a check for whether the extension is currently initialised

Thanks, have moved down

jakubcolony · 2023-02-14T18:35:52Z

src/types.ts

+/*
+ * Contract calls
+ */
+export enum ColonyActions {


Could the name be more indicative that these are the contract calls?

I've gone for ColonyOperations. Preferable?

jakubcolony · 2023-02-14T18:36:19Z

src/types.ts

+export enum ColonyMotionType {
+  MintTokensMotion = 'MINT_TOKENS_MOTION',
+}


I noticed in the corresponding CDapp PR you keep those merged, should we also stick to that idea here?

rdig

Left some comments, but otherwise seems good.

rdig · 2023-02-15T16:43:16Z

src/eventProcessor.ts

-        'tokens were minted in colony:',
-        colonyAddress,
-      );
+      if (amount && amount.toString() !== '0') {


Is the motion set to very short timeout ? Ie: it finishes instantly ? Otherwise I can't really explain this.

The underlying action / event doesn't get triggered until the motion finishes (and that is done manually).

If that isn't the case I don't really understand what's going on. Maybe ask @area for his input ?

rdig · 2023-02-15T16:44:24Z

src/index.ts

@@ -61,7 +61,7 @@ const startIngestor = async (): Promise<void> => {
  /*
   * Setup all listeners we care about
   */
-  await blockListener();


Not saying it's something wrong with this, I'm just curious on why you removed it

This one's not an async function

@rdig I think you asked this on another pr as well so just tagging you here in case you missed my response. Not using await since there's nothing to await.

rdig · 2023-02-15T16:44:51Z

src/trackExtensions.ts

+    if (Extension.VotingReputation === extensionId) {
+      addMotionEventListener(ContractEventsSignatures.MotionCreated, colony);
+    }


rdig · 2023-02-15T16:48:11Z

src/utils/motions.ts

+
+  try {
+    return colonyClient.interface.parseTransaction({
+      data: motion.action,


If I remember correctly the motion's data (action) is available in the event, and if that is the case we don't have to call getMotion to get it anymore.

Please check if that's the case, I might be wrong though.

@rdig I don't think so, the event emits motionId, creator, and domainId but not the underlying action

willm30 · 2023-03-02T13:04:16Z

src/utils/events.ts

- * It basically does away with all the boilerplate of setting up topics, setting
- * the event listener, parsing the received event
- */
-export const eventListenerGenerator = async (


Moved to own file as I have added a few helpers

willm30 · 2023-03-02T13:04:44Z

src/utils/events.ts

- * Network Client specific event listener,
- * which uses `eventListenerGenerator` under the hood
- */
-export const addNetworkEventListener = async (


moved to '~utils/eventListeners'

willm30 · 2023-03-02T13:06:58Z

src/utils/eventListeners/eventListenerGenerator.ts

+    contractAddress,
+  );
+
+  const filter = getEventFilter(eventSignature, contractAddress, clientType);


Wrote some helpers for this. Should make it easier to extend.

willm30 · 2023-03-02T13:13:32Z

@jakubcolony Have rebased this to account for changes to event processor

jakubcolony · 2023-03-08T17:17:51Z

@willm30 I pulled your latest changes and I'm seeing those errors when trying to create mint tokens motion. Looks like you're trying to write some fields (tokenSymbol, fromDomain) that have been modified since:

errors: [
    {
      message: 'Variable "$input" got invalid value { id: "0x97e6da42f5d3b2d5e02a7288d5b0d222322c72fe5bdfa6aba36547eeed8f868f", colonyId: "0xA4b4F5d9DBe383206Bd11C0e447F4462B9ea6cAE", type: "MINT_TOKENS_MOTION", isMotion: true, tokenSymbol: "JKB", fromDomain: "0xA4b4F5d9DBe383206Bd11C0e447F4462B9ea6cAE_1", initiatorAddress: "0xb77D57F4959eAfA0339424b83FcFaf9c15407461", amount: "1", blockNumber: 1100, tokenAddress: "0x8c60Ab84fa41a5E826BB91C30680e3b5faF8972B" }; Field "tokenSymbol" is not defined by type CreateColonyActionInput.',
      locations: [Array]
    },
    {
      message: 'Variable "$input" got invalid value { id: "0x97e6da42f5d3b2d5e02a7288d5b0d222322c72fe5bdfa6aba36547eeed8f868f", colonyId: "0xA4b4F5d9DBe383206Bd11C0e447F4462B9ea6cAE", type: "MINT_TOKENS_MOTION", isMotion: true, tokenSymbol: "JKB", fromDomain: "0xA4b4F5d9DBe383206Bd11C0e447F4462B9ea6cAE_1", initiatorAddress: "0xb77D57F4959eAfA0339424b83FcFaf9c15407461", amount: "1", blockNumber: 1100, tokenAddress: "0x8c60Ab84fa41a5E826BB91C30680e3b5faF8972B" }; Field "fromDomain" is not defined by type CreateColonyActionInput. Did you mean fromDomainId or toDomainId?',
      locations: [Array]
    }
  ]

willm30 · 2023-03-09T12:43:54Z

@jakubcolony Yep I will need to update this as I rebased the cdapp branch.

jakubcolony

@willm30 Nice work overall, but after pulling the latest from port/colony-motions I can't create a colony to test this, due to the following error from GetFullColonyByName query:

"Cannot query field \"color\" on type \"Domain\". Did you mean \"colony\" or \"colonyId\"?"

Also, we don't have a linter yet, but do you have an eslint extension installed? I'm seeing a few eslint warnings here and there, e.g. utils/motions.ts:

jakubcolony · 2023-03-10T18:34:53Z

src/handlers/ motions/index.ts

@@ -0,0 +1 @@
+export { default as handleMotionCreated } from './motionCreated';


Looks like there's an extra space before motions:

jakubcolony · 2023-03-10T18:36:19Z

src/handlers/ motions/motionCreated.ts

+import {
+  getParsedActionFromMotion,
+  writeMintTokensMotionToDB,
+} from '~utils/motions';


You could export the motions stuff from ~utils and import it together with verbose

jakubcolony · 2023-03-10T18:53:49Z

src/utils/domains.ts

+export const formatDomain = (colonyAddress: string, domainId: string) =>
+  `${colonyAddress}_${domainId.toString()}`;


Just a heads up, there should be a getDomainDatabaseId util in port/actions

jakubcolony · 2023-03-10T18:56:40Z

src/utils/index.ts

@@ -5,3 +5,4 @@ export * from './extensions';
 export * from './numbers';
 export * from './tokens';
 export * from './actions';
+export * from './eventListeners';


Matter of personal preferece I guess, but I would consider changing utils/events.ts to events directory and sticking all your extracted utils in there. events and eventListeners seem closely tied

jakubcolony · 2023-03-10T19:00:27Z

src/utils/tokens.ts

+export const getColonyTokenData = async (
+  tokenAddress: string,
+  colonyClient: AnyColonyClient,
+): Promise<{ name: string; decimals: number; symbol: string }> => {
+  const tokenClient = await getTokenClient(tokenAddress, colonyClient.provider);
+  return await tokenClient.getTokenInfo();
+};


Is this used anywhere? I can see it imported in utils/motions but doesn't seem to be called

willm30 · 2023-03-16T16:38:59Z

@jakubcolony I have rebased the cdapp after you rebased actions and have rebased this branch too on latest actions. I'm not experiencing problems creating a colony and all seems to have been integrated correctly. But let me know if after building both branches afresh, you still have issues.

jakubcolony

Nice work! 💪

jakubcolony · 2023-03-17T14:26:03Z

src/utils/motions.ts

+  parsedAction: TransactionDescription,
+) => {
+  const { name, args: actionArgs } = parsedAction;
+  const amount = actionArgs[0].toString();


Could this element of the array be undefined? If so, consider adding optional chaining

@jakubcolony No, don't expect so. The front end should not be passing through an undefined amount of tokens to mint.

It maps to the motion params field in the current set up, for reference:

Is that a safe assumption to make, that it will never be broken by anyone?
What about calling it directly from a contract?

@jakubcolony I don't believe calling it directly from a contract will be a problem either. You need to pass an "encoded action" to the createMotion call, and in order for the encode action function to succeed, it must receive a complete set of arguments to the underlying action call (by 'complete' I mean correct length, each of which is the correct type).

@rdig Can a malformed event find its way into the block ingestor? I.e. an event without the expected set of arguments?

No, not really. If that happens it'll be the event of the year on ethereum :)

While I approve of your proactive thinking, we shouldn't be worrying about this for now

Good to know, thanks 👍

willm30 changed the base branch from master to port/motions February 10, 2023 20:48

willm30 force-pushed the add/root-motion branch 6 times, most recently from 2b6d12f to 8e6a07a Compare February 13, 2023 18:41

willm30 self-assigned this Feb 13, 2023

willm30 force-pushed the add/root-motion branch from 8e6a07a to 5940d67 Compare February 13, 2023 19:16

willm30 commented Feb 13, 2023

View reviewed changes

willm30 force-pushed the add/root-motion branch 2 times, most recently from 5890066 to 32f56c9 Compare February 14, 2023 12:14

willm30 commented Feb 14, 2023

View reviewed changes

willm30 marked this pull request as ready for review February 14, 2023 12:15

willm30 requested a review from a team February 14, 2023 12:16

willm30 mentioned this pull request Feb 14, 2023

Port root motion saga JoinColony/colonyCDapp#261

Merged

5 tasks

jakubcolony reviewed Feb 14, 2023

View reviewed changes

rdig approved these changes Feb 15, 2023

View reviewed changes

willm30 force-pushed the add/root-motion branch 2 times, most recently from e042c49 to 70c25ed Compare February 27, 2023 13:16

willm30 force-pushed the port/motions branch from e74980d to 63d81b6 Compare March 2, 2023 12:03

willm30 force-pushed the add/root-motion branch 2 times, most recently from c3aa3fb to 5ea6b1d Compare March 2, 2023 13:03

willm30 commented Mar 2, 2023

View reviewed changes

willm30 requested a review from jakubcolony March 2, 2023 13:13

willm30 force-pushed the add/root-motion branch from 5ea6b1d to 43ef471 Compare March 10, 2023 10:38

jakubcolony requested changes Mar 10, 2023

View reviewed changes

willm30 force-pushed the port/motions branch from 63d81b6 to 47d2aa4 Compare March 16, 2023 15:47

willm30 force-pushed the add/root-motion branch from 43ef471 to 201d984 Compare March 16, 2023 15:50

willm30 added 2 commits March 16, 2023 16:56

Add: root motion support

5811cb6

Refactor: eventListenerGenerator

6dcf4e2

willm30 force-pushed the add/root-motion branch from 201d984 to 6dcf4e2 Compare March 16, 2023 15:56

Chore: clean up after rebase

b950185

willm30 requested a review from jakubcolony March 16, 2023 16:37

jakubcolony approved these changes Mar 17, 2023

View reviewed changes

willm30 merged commit b4fc7dc into port/motions Mar 17, 2023

willm30 deleted the add/root-motion branch March 17, 2023 17:54

		export const formatDomain = (colonyAddress: string, domainId: string) =>
		`${colonyAddress}_${domainId.toString()}`;

		@@ -0,0 +1 @@
		export { default as handleMotionCreated } from './motionCreated';

Add mint tokens motion support #16

Add mint tokens motion support #16

Conversation

willm30 commented Feb 10, 2023 • edited Loading

Description

Testing

Notes

Troubleshooting

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakubcolony left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rdig left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

willm30 commented Mar 2, 2023

jakubcolony commented Mar 8, 2023

willm30 commented Mar 9, 2023

jakubcolony left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

willm30 commented Mar 16, 2023 • edited Loading

jakubcolony left a comment

Choose a reason for hiding this comment

jakubcolony Mar 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

willm30 commented Feb 10, 2023 •

edited

Loading

willm30 commented Mar 16, 2023 •

edited

Loading

jakubcolony Mar 17, 2023 •

edited

Loading