Implement the new trigger and generic task node structure #35

v9n · 2024-11-11T19:47:18Z

Implement change to our task trigger/node https://docs.google.com/document/d/1DM0VJ0hUnC4FIWAbNSKAaLfQ3U7EI3GRd0DcQvUA8B8/edit?pli=1&tab=t.0#heading=h.oi6mw4tp84nj

This PR mainly handles 2 things:

Define the new trigger with time/block/event/manual
Add the new edges field to maintain relationship between nodes
Remove next from nodes. Edges handles it now.

Will update https://github.com/AvaProtocol/ava-sdk-js/pulls next to conform to the new protobuf and add new method.

An example task.

n example get '01JCK4XF8YRKAAM00KS96HD4JM'
Inspect Task ID:  01JCK4XF8YRKAAM00KS96HD4JM

edges:                    [
  {
    "id": "edge-123abcdef",
    "start": "transfer_erc20_1",
    "end": ""
  }
]
executions:               []
id:                       {
  "bytes": "01JCK4XF8YRKAAM00KS96HD4JM"
}
owner:                    "0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5"
smart_wallet_address:     "0xdD85693fd14b522a819CC669D6bA388B4FCd158d"
trigger:                  {
  "trigger_type": "BlockTrigger",
  "block": {
    "interval": "5"
  },
  "trigger_condition": "block"
}
start_at:                 "1731514544"
expired_at:               "1734106514"
memo:                     "Demo Example task for 0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5"
completed_at:             "0"
status:                   "Active"
recurring:                false
node_type:                "ETHTransferTask"
id:                       "get_oracle_price"
branch:                   {
  "elseIfs": [],
  "if": {
    "expression": "bigCmp(priceChainlink(\"0x694AA1769357215DE4FAC081bf1f309aDC325306\"),toBigInt(\"10000\") > 0",
    "next": "transfer_erc20_1"
  },
  "else": null
}
task_body:                "branch"
node_type:                "ETHTransferTask"
id:                       "transfer_erc20_1"
name:                     "Transfer Test Token"
contract_write:           {
  "contract_address": "0x2e8bdb63d09ef989a0018eeb1c47ef84e3e61f7b",
  "call_data": "0xa9059cbb000000000000000000000000e0f7d11fd714674722d325cd86062a5f1882e13a000000000000000000000000000000000000000000000000a688906bd8b00000",
  "contract_abi": ""
}
task_body:                "contract_write"

another example with fixed schedule

Inspect Task ID:  01JCJJ6RMMJTN5XYQTG347HYQZ

edges:                    [
  {
    "id": "edge-123abcdef",
    "start": "transfer_erc20_1",
    "end": ""
  }
]
executions:               []
id:                       {
  "bytes": "01JCJJ6RMMJTN5XYQTG347HYQZ"
}
owner:                    "0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5"
smart_wallet_address:     "0xdD85693fd14b522a819CC669D6bA388B4FCd158d"
trigger:                  {
  "trigger_type": "FixedEpochTrigger",
  "at": {
    "epoches": [
      "1731498496",
      "1731502096"
    ]
  },
  "trigger_condition": "at"
}
start_at:                 "1731494926"
expired_at:               "1734086896"
memo:                     "Demo Example task for 0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5"
completed_at:             "0"
status:                   "Active"
recurring:                false
node_type:                "ETHTransferTask"
id:                       "get_oracle_price"
branch:                   {
  "elseIfs": [],
  "if": {
    "expression": "bigCmp(priceChainlink(\"0x694AA1769357215DE4FAC081bf1f309aDC325306\"),toBigInt(\"10000\") > 0",
    "next": "transfer_erc20_1"
  },
  "else": null
}
task_body:                "branch"
node_type:                "ETHTransferTask"
id:                       "transfer_erc20_1"
name:                     "Transfer Test Token"
contract_write:           {
  "contract_address": "0x2e8bdb63d09ef989a0018eeb1c47ef84e3e61f7b",
  "call_data": "0xa9059cbb000000000000000000000000e0f7d11fd714674722d325cd86062a5f1882e13a000000000000000000000000000000000000000000000000a688906bd8b00000",
  "contract_abi": ""
}
task_body:                "contract_write"

another one with cron:

Inspect Task ID:  01JCK500SEF8NWVNCPTAWHQK82

edges:                    [
  {
    "id": "edge-123abcdef",
    "start": "transfer_erc20_1",
    "end": ""
  }
]
executions:               []
id:                       {
  "bytes": "01JCK500SEF8NWVNCPTAWHQK82"
}
owner:                    "0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5"
smart_wallet_address:     "0xdD85693fd14b522a819CC669D6bA388B4FCd158d"
trigger:                  {
  "trigger_type": "ManualTrigger",
  "cron": {
    "cron_table": [
      "0 */5 * * *"
    ]
  },
  "trigger_condition": "cron"
}
start_at:                 "1731514628"
expired_at:               "1734106598"
memo:                     "Demo Example task for 0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5"
completed_at:             "0"
status:                   "Active"
recurring:                false
node_type:                "ETHTransferTask"
id:                       "get_oracle_price"
branch:                   {
  "elseIfs": [],
  "if": {
    "expression": "bigCmp(priceChainlink(\"0x694AA1769357215DE4FAC081bf1f309aDC325306\"),toBigInt(\"10000\") > 0",
    "next": "transfer_erc20_1"
  },
  "else": null
}
task_body:                "branch"
node_type:                "ETHTransferTask"
id:                       "transfer_erc20_1"
name:                     "Transfer Test Token"
contract_write:           {
  "contract_address": "0x2e8bdb63d09ef989a0018eeb1c47ef84e3e61f7b",
  "call_data": "0xa9059cbb000000000000000000000000e0f7d11fd714674722d325cd86062a5f1882e13a000000000000000000000000000000000000000000000000a688906bd8b00000",
  "contract_abi": ""
}
task_body:                "contract_write"

socket-security · 2024-11-14T00:46:32Z

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/[email protected]	None	`0`	65.2 kB	morninded

View full report↗︎

chrisli30

Added my comments to the current design.

chrisli30 · 2024-11-14T01:37:37Z

protobuf/avs.proto

+}
+
+message TaskNode {
+  TaskType node_type = 1;


Match the variable name node_type with the type name TaskType. In my opinion we can simplify the names with the followings:

message TaskNode -> message Node
TaskType -> NodeType, and make it NodeType type = 1

Match the variable name node_type with the type name TaskType
Yes, will change to task_type

In my opinion we can simplify the names with the followings:
message TaskNode -> message Node
TaskType -> NodeType, and make it NodeType type = 1

I actually name it on purpose

Node is too generic in our field. We have node in list, we have node in server. It's like calling something Item. Because of that, I want to avoid any confusion down the line, and explicitly calling it TaskNode. it's also more grep-able if we have to search code.

type I want to avoid the case when we access it like this something.type just want to avoid someone accidently think this maybe the type of the object/class. And also same with searchable. .node_type

let me know if you still think in redundant, i can change.

okay, naming them compresive works. It’s just a naming convension.

Also, I was asking about TaskType node_type = 1;, that maybe we should calle it TaskType task_type = 1; for consistency?

chrisli30 · 2024-11-14T01:39:54Z

protobuf/avs.proto

+  string name = 3;
+
+  // based on node_type one and only one of these field are set
+  oneof task_body {


The TaskType already defines these categories, and the task_body values seem redundant, but let me get a screenshot from the Studio for furthur clarification.

This is a screenshot of the current Node types in Studio. As you can see I’m grouping them by functions.

The TaskType already defines these categories, and the task_body values seem redundant,

task_body is relevant to protobuf so it can decode the object becasue we're using oneof here
This is one-off https://protobuf.dev/programming-guides/proto3/#oneof

oneof take the value of field name by default so I was trying to be more user fiendly and having the extra node_type.

Let me just merge them into one so it won't have this dup.

protobuf/avs.proto

chrisli30 · 2024-11-14T01:43:01Z

model/task.go

+		Task: &avsproto.Task{
+			Id: &avsproto.UUID{
+				Bytes: taskID,
+			},


Isn’t id just a ulid string? I think we should just have id: string
instead of return a nested structure id: { bytes: '01JCKCRTZBS158BRP8E42P2DQW' }. This nested format doesn’t provide any real benefit to the user and only complicates retrieving the string representation of the ID.

Besides, I am using the ulid on the frontend, and find the upper cases are a little hard to read, should we return the id in lower case?

Comparison:
uppercases:
{ id: '01JCM1TGR85E59ZYEG5Q7WHKZ6' }

lowercases:
{ id: '01jcm1tgr85e59zyeg5q7whkz6' }

Isn’t id just a ulid string? I think we should just have id: string

uuid/ulid is actually a byte array. string is a way to re-present it in human readable form, aka canonical form.

I used []byte before (thus the name) but it's not easy to enter []byte in the gRPC UI thus I resorted to using string so ppl can type it out

Let me update to direct string instead.

Besides, I am using the ulid on the frontend, and find the upper cases are a little hard to read, should we return the id in lower case?

You can display uppercase/lowecase on the front-end. doesn't matter. the alphabet doesn't have lowecase. server side will convert it eventually.

okay, let’s keep the upper case for the id, so we don’t care about conversion on the server side.

yeah, bytes have to be converted to string for transmission, so I think we should remove the bytes property key, to achieve { id: '01JCM1TGR85E59ZYEG5Q7WHKZ6' }

chrisli30 · 2024-11-14T01:45:21Z

protobuf/avs.proto

+
+  // repeatable means a task can continue to run even after the first execution.
+  // By default, recurring is false, task will only executed once.
+  bool recurring  = 10;


If it’s a boolean varialbe, I suggest that we name it is_recurring, instead of recurring. At least in javascript that norm is with a prefix is.

will change

okay, we have made a decison about this last year. Checking out the automation hub, I think the below set up makes more sense.

maxExeCount: number
0 - good till cancel, equals to is_recurring
1, 2, 3, .... - the actual execution count

The reason being that with is_recurring: bool we couldn’t limit the number of execution needed for a reccuring job.

Slack workflow called it Frequency:

protobuf/avs.proto

Formatted console.log output of example.js

v9n · 2024-11-20T09:46:41Z

@chrisli30 Ready to review

chrisli30

Cood looks good. I like the encapsulation of Conditions in the examples/static_codegen/avs_pb.js

v9n marked this pull request as draft November 11, 2024 19:47

v9n mentioned this pull request Nov 11, 2024

New task management #34

Merged

v9n force-pushed the new-task-data-structure branch from 05ea0ba to 8ce3e99 Compare November 11, 2024 19:54

v9n marked this pull request as ready for review November 12, 2024 18:40

v9n changed the title ~~New task data structure~~ Implement the trigger and generic task actions to avoid null field Nov 12, 2024

v9n changed the title ~~Implement the trigger and generic task actions to avoid null field~~ Implement the new trigger and generic task node structure Nov 12, 2024

v9n marked this pull request as draft November 12, 2024 18:42

v9n requested a review from chrisli30 November 13, 2024 10:06

v9n marked this pull request as ready for review November 13, 2024 10:06

v9n added 2 commits November 13, 2024 16:45

convert magic error msg to const

7ea1dee

implement the new trigger, node, edge structure

460df3c

v9n force-pushed the new-task-data-structure branch from 04c8478 to 460df3c Compare November 14, 2024 00:46

chrisli30 added 3 commits November 13, 2024 17:14

Formatted console.log output of example.js

c4a5746

Revert part of last commmit to keep in sync with parent branch

2c554da

Updated example.js wallet command

a16402d

chrisli30 reviewed Nov 14, 2024

View reviewed changes

v9n added 3 commits November 19, 2024 09:50

Merge pull request #37 from AvaProtocol/chris-update_example_logs

c365d66

Formatted console.log output of example.js

iteration

8b944b7

add unit test

a85ec40

v9n had a problem deploying to Test November 19, 2024 20:25 — with GitHub Actions Failure

remove contract test

d5a2d05

v9n had a problem deploying to Test November 19, 2024 20:27 — with GitHub Actions Failure

v9n added 2 commits November 19, 2024 12:30

run a subset of test

bd0e3c4

update comment

24aa406

v9n had a problem deploying to Test November 19, 2024 20:31 — with GitHub Actions Failure

update docs

a18adde

v9n had a problem deploying to Test November 19, 2024 20:37 — with GitHub Actions Failure

update pr workflow

ea84ab0

v9n temporarily deployed to Test November 19, 2024 20:39 — with GitHub Actions Inactive

v9n added 2 commits November 19, 2024 18:59

update tag

511d246

error handling

b138fe3

v9n temporarily deployed to Test November 20, 2024 03:00 — with GitHub Actions Inactive

lint dockerfile

369e281

v9n temporarily deployed to Test November 20, 2024 03:03 — with GitHub Actions Inactive

run http latest

22bd427

v9n temporarily deployed to Test November 20, 2024 03:14 — with GitHub Actions Inactive

v9n added 2 commits November 19, 2024 21:19

improve error and add cache

9ce3dae

update unstable build

c14b147

v9n temporarily deployed to Test November 20, 2024 05:20 — with GitHub Actions Inactive

v9n added 2 commits November 19, 2024 21:27

return right error

965b81e

add bigcache

7f64198

v9n temporarily deployed to Test November 20, 2024 05:27 — with GitHub Actions Inactive

update

79d9183

v9n temporarily deployed to Test November 20, 2024 09:22 — with GitHub Actions Inactive

chrisli30 approved these changes Nov 20, 2024

View reviewed changes

only public latest on main

caa0d27

v9n temporarily deployed to Test November 20, 2024 19:18 — with GitHub Actions Inactive

v9n merged commit dc02014 into main Nov 20, 2024
4 checks passed

v9n deleted the new-task-data-structure branch November 20, 2024 23:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement the new trigger and generic task node structure #35

Implement the new trigger and generic task node structure #35

v9n commented Nov 11, 2024 •

edited

Loading

socket-security bot commented Nov 14, 2024 •

edited

Loading

chrisli30 left a comment

chrisli30 Nov 14, 2024

v9n Nov 14, 2024

chrisli30 Nov 14, 2024

chrisli30 Nov 14, 2024

chrisli30 Nov 14, 2024

v9n Nov 14, 2024

chrisli30 Nov 14, 2024

chrisli30 Nov 14, 2024

v9n Nov 14, 2024

chrisli30 Nov 14, 2024 •

edited

Loading

chrisli30 Nov 14, 2024

v9n Nov 14, 2024

chrisli30 Nov 14, 2024

v9n commented Nov 20, 2024

chrisli30 left a comment

Implement the new trigger and generic task node structure #35

Implement the new trigger and generic task node structure #35

Conversation

v9n commented Nov 11, 2024 • edited Loading

socket-security bot commented Nov 14, 2024 • edited Loading

chrisli30 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

chrisli30 Nov 14, 2024 • 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

v9n commented Nov 20, 2024

chrisli30 left a comment

Choose a reason for hiding this comment

v9n commented Nov 11, 2024 •

edited

Loading

socket-security bot commented Nov 14, 2024 •

edited

Loading

chrisli30 Nov 14, 2024 •

edited

Loading