feat: implement vm message extraction #1027

frrist · 2022-08-02T21:19:57Z

What

This PR addresses #978 by:

Implementing a vm_messages model and schema for tracking internal messages (off-chain) sent by the VM during on-chain message execution
Modifies lily to extract messages sent by the VM.

An example of the queries this allows:

Select a message sent to a multisig actor that proposes sending a method with the below params

select *
from parsed_messages
where cid = 'bafy2bzacec4mncfpdvu4rl4szr74qajty3wa6jxwallgfl4w2quf6hzfd5m42'

cid	height	from	to	value	method	params
bafy2bzacec4mncfpdvu4rl4szr74qajty3wa6jxwallgfl4w2quf6hzfd5m42	2020710	f1s5fmqz3aa4mvmoi235jarkj2f2xxuymrhjxochi	f2ltn3tgp2a7uldxn2fgftzmodfcjtcu655p3vjmi	0	Propose	{"To": "f01848682", "Value": {"Int": 0}, "Method": 16, "Params": "gUoAAXuoIZfGfo54"}

Select all VM messages (via source) resulting from the execution of this proposed method.

select *
from vm_messages
where source = 'bafy2bzacec4mncfpdvu4rl4szr74qajty3wa6jxwallgfl4w2quf6hzfd5m42'

height	state_root	cid	source	from	to	value	method	actor_code	exit_code	gas_used	params	returns
2020710	bafy2bzacedb66jfje3d6pcy64hnpdxuurkvl5p36n27axjg7dpk6ohoj3xoyu	bafy2bzaceaixcm7hv6qtgqb7ccw7tayxcu64c4qcdcqtf332uk3vrwol6ozrw	bafy2bzacec4mncfpdvu4rl4szr74qajty3wa6jxwallgfl4w2quf6hzfd5m42	f01833148	f01848682	0	16	bafk2bzacecgnynvd3tene3bvqoknuspit56canij5bpra6wl4mrq2mxxwriyu	0	0	{"AmountRequested": {"Int": 27357152872216039032}}	"27357152872216039032"
2020710	bafy2bzacedb66jfje3d6pcy64hnpdxuurkvl5p36n27axjg7dpk6ohoj3xoyu	bafy2bzacebhlcwggejv4mgb6hgvoqmxvaouzpysdtdwlgg4gnbdgqnzkwak72	bafy2bzacec4mncfpdvu4rl4szr74qajty3wa6jxwallgfl4w2quf6hzfd5m42	f01848682	f01833148	27357152872216039032	0	bafk2bzacebhldfjuy4o5v7amrhp5p2gzv2qo5275jut4adnbyp56fxkwy5fag	0	0	NULL	NULL

We see that the proposed message was used to withdraw a balance of from miner actor by calling the miners WithdrawBalance method (via bafy2bzaceaixcm7hv6qtgqb7ccw7tayxcu64c4qcdcqtf332uk3vrwol6ozrw), which then resulted in the miner actor performing a send of the requested amount (via bafy2bzacebhlcwggejv4mgb6hgvoqmxvaouzpysdtdwlgg4gnbdgqnzkwak72)

codecov-commenter · 2022-08-03T02:01:40Z

Codecov Report

Merging #1027 (b6b937f) into master (ead40c9) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@          Coverage Diff           @@
##           master   #1027   +/-   ##
======================================
  Coverage    35.6%   35.6%           
======================================
  Files          44      44           
  Lines        2879    2881    +2     
======================================
+ Hits         1025    1027    +2     
  Misses       1750    1750           
  Partials      104     104

frrist · 2022-08-03T18:16:41Z

schemas/v1/8_vm_messages.go

+ALTER TABLE ONLY {{ .SchemaName | default "public"}}.vm_messages ADD CONSTRAINT vm_messages_pkey PRIMARY KEY (height, state_root, cid, source);
+CREATE INDEX vm_messages_height_idx ON {{ .SchemaName | default "public"}}.vm_messages USING BRIN (height);
+CREATE INDEX vm_messages_cid_idx ON {{ .SchemaName | default "public"}}.vm_messages USING HASH (cid);
+CREATE INDEX vm_messages_source_idx ON {{ .SchemaName | default "public"}}.vm_messages USING HASH (source);
+CREATE INDEX vm_messages_from_idx ON {{ .SchemaName | default "public"}}.vm_messages USING HASH ("from");
+CREATE INDEX vm_messages_to_idx ON {{ .SchemaName | default "public"}}.vm_messages USING HASH ("to");
+CREATE INDEX vm_messages_actor_code_method_idx ON {{ .SchemaName | default "public"}}.vm_messages USING BTREE (actor_code, method);


@kasteph @placer14 here are the index definitions we discussed in #978, please review.

placer14

Sticky approve. Nice and clean! 🤝

placer14 · 2022-08-04T14:36:05Z

chain/indexer/tasktype/table_tasks.go

@@ -178,8 +182,9 @@ var TableFieldComments = map[string]map[string]string{
 		"DealID":               "Identifier for the deal.",
 		"EndEpoch":             "The epoch at which this deal with end.",
 		"Height":               "Epoch at which this deal proposal was added or changed.",
+		"IsString":             "Related to FIP: https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0027.md",


I don't want to read the FIP to get the gist.

Suggested change

"IsString": "Related to FIP: https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0027.md",

"IsString": "Indicates if the value of Label is a string. Assume binary blob otherwise. Related to FIP: https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0027.md",

placer14 · 2022-08-04T14:37:17Z

chain/indexer/tasktype/table_tasks.go

@@ -197,6 +202,21 @@ var TableFieldComments = map[string]map[string]string{
 	ParsedMessage:         {},
 	InternalMessage:       {},
 	InternalParsedMessage: {},
+	VmMessage: {
+		"ActorCode": "ActorCode of To (receiver)",


Suggested change

"ActorCode": "ActorCode of To (receiver)",

"ActorCode": "ActorCode of To (receiver).",

placer14 · 2022-08-04T14:52:45Z

model/messages/vm.go

+	// Cid of the message.
+	Cid string `pg:",pk,notnull"`
+	// On-chain message triggering the message.
+	Source string `pg:",pk,notnull"`


I don't think we need source to be included in the primary key. cid should have enough uniqueness within a height/stateroot.

Suggested change

Source string `pg:",pk,notnull"`

Source string `pg:",notnull"`

Source must remain a primary key as VM message CIDs can conflict at the same height/stateroot.

Isn't cid the CID of the intermediate (internal) message? In other words, one source has many cids? If so, then the index should go on the column with the higher cardinality (between cid and source) which I expect to be the intermediate message CIDs and I understand that to be the cid column.

Isn't cid the CID of the intermediate (internal) message, In other words, one source has many cids?

Yes correct.

Are you proposing we drop source as a primary key?

Yes. Just remove from the PK to reduce the size of that index. We can use the standalone hash index for source if we ever need to condition on that column. It should reduce the size of the PK index without any performance cost.

Okay. After some discussion w @frrist, it seems that the cid is not unique and so source is required in the PK afterall. 😢

I would probably recommend the removal of the hash-based source index. I think both indices are redundant and if the PK index isn't getting a trim, let's drop the other.

+1 on dropping the source hash index esp if it's part of the PK.

placer14 · 2022-08-04T14:53:16Z

model/messages/vm.go

+	// Value attoFIL contained in message.
+	Value string `pg:"type:numeric,notnull"`
+	// Method called on To (receiver)
+	Method uint64 `pg:",use_zero"`


Not null? (I know this is inconsequential given we write our own migrations, but it's good context that doesn't hurt to include and keeps consistency w other fields usage of the same tag.)

Suggested change

Method uint64 `pg:",use_zero"`

Method uint64 `pg:",notnull,use_zero"`

placer14 · 2022-08-04T14:53:55Z

model/messages/vm.go

+	// ActorCode of To (receiver)
+	ActorCode string `pg:",notnull"`
+	// ExitCode of message execution.
+	ExitCode int64 `pg:",use_zero"`


Suggested change

ExitCode int64 `pg:",use_zero"`

ExitCode int64 `pg:",notnull,use_zero"`

placer14 · 2022-08-04T14:54:02Z

model/messages/vm.go

+	// ExitCode of message execution.
+	ExitCode int64 `pg:",use_zero"`
+	// GasUsed by message.
+	GasUsed int64 `pg:",use_zero"`


Suggested change

GasUsed int64 `pg:",use_zero"`

GasUsed int64 `pg:",notnull,use_zero"`

schemas/v1/8_vm_messages.go

placer14 · 2022-08-04T16:05:29Z

schemas/v1/8_vm_messages.go

+	returns jsonb
+);
+ALTER TABLE ONLY {{ .SchemaName | default "public"}}.vm_messages ADD CONSTRAINT vm_messages_pkey PRIMARY KEY (height, state_root, cid, source);
+CREATE INDEX vm_messages_height_idx ON {{ .SchemaName | default "public"}}.vm_messages USING BRIN (height);


I think BRIN indexing is dependent on write order, which is use-case specific. BTREE is the better generalized index here.

Suggested change

CREATE INDEX vm_messages_height_idx ON {{ .SchemaName | default "public"}}.vm_messages USING BRIN (height);

CREATE INDEX vm_messages_height_idx ON {{ .SchemaName | default "public"}}.vm_messages USING BTREE (height);

Indeed, will use btree instead.

placer14 · 2022-08-04T16:06:18Z

schemas/v1/8_vm_messages.go

+COMMENT ON COLUMN {{ .SchemaName | default "public"}}.vm_messages.method IS 'The method number invoked on the recipient actor. Only unique to the actor the method is being invoked on. A method number of 0 is a plain token transfer - no method execution';
+COMMENT ON COLUMN {{ .SchemaName | default "public"}}.vm_messages.actor_code IS 'The CID of the actor that received the message.';
+COMMENT ON COLUMN {{ .SchemaName | default "public"}}.vm_messages.exit_code IS 'The exit code that was returned as a result of executing the message.';
+COMMENT ON COLUMN {{ .SchemaName | default "public"}}.vm_messages.gas_used IS 'A measure of the amount of resources (or units of gas) consumed, in order to execute a message.';


Curious: Do you know what resources this would be other than gas?

Reusing the description of gas here that we use elsewhere, e.g.: https://github.com/filecoin-project/lily/blob/v0.10.0/schemas/v1/base.go#L391

I suspected it was copypasta. Just wondering if there is anything other than gas that it would have been referring to and couldn't think of.

- use notnull

kasteph · 2022-08-08T13:21:50Z

schemas/v1/8_vm_messages.go

+CREATE INDEX vm_messages_height_idx ON {{ .SchemaName | default "public"}}.vm_messages USING BTREE (height);
+CREATE INDEX vm_messages_cid_idx ON {{ .SchemaName | default "public"}}.vm_messages USING HASH (cid);
+CREATE INDEX vm_messages_source_idx ON {{ .SchemaName | default "public"}}.vm_messages USING HASH (source);
+CREATE INDEX vm_messages_from_idx ON {{ .SchemaName | default "public"}}.vm_messages USING HASH ("from");
+CREATE INDEX vm_messages_to_idx ON {{ .SchemaName | default "public"}}.vm_messages USING HASH ("to");
+CREATE INDEX vm_messages_actor_code_method_idx ON {{ .SchemaName | default "public"}}.vm_messages USING BTREE (actor_code, method);


If we want to do a write-optimized schema, I'd recommend to get rid of several of these indices since all these indices need to be updated whenever an insert or update happens.

I propose the following:

Suggested change

CREATE INDEX vm_messages_height_idx ON {{ .SchemaName | default "public"}}.vm_messages USING BTREE (height);

CREATE INDEX vm_messages_cid_idx ON {{ .SchemaName | default "public"}}.vm_messages USING HASH (cid);

CREATE INDEX vm_messages_source_idx ON {{ .SchemaName | default "public"}}.vm_messages USING HASH (source);

CREATE INDEX vm_messages_from_idx ON {{ .SchemaName | default "public"}}.vm_messages USING HASH ("from");

CREATE INDEX vm_messages_to_idx ON {{ .SchemaName | default "public"}}.vm_messages USING HASH ("to");

CREATE INDEX vm_messages_actor_code_method_idx ON {{ .SchemaName | default "public"}}.vm_messages USING BTREE (actor_code, method);

CREATE INDEX vm_messages_height_idx ON {{ .SchemaName | default "public"}}.vm_messages USING BTREE (height);

CREATE INDEX vm_messages_from_idx ON {{ .SchemaName | default "public"}}.vm_messages USING HASH ("from");

CREATE INDEX vm_messages_to_idx ON {{ .SchemaName | default "public"}}.vm_messages USING HASH ("to");

Regarding:

CREATE INDEX vm_messages_actor_code_method_idx ON {{ .SchemaName | default "public"}}.vm_messages USING BTREE (actor_code, method);

This only makes sense if we have queries that have a filter that's where actor_code = x and method = b or where actor_code = x . This wouldn't work well with filters: where method = b and actor_code = x or where method = b.

Therefore, I'd recommend based on Mike's comment here:

CREATE INDEX vm_messages_actor_code_idx ON {{ .SchemaName | default "public"}}.vm_messages USING BTREE (actor_code);

This only makes sense if we have queries that have a filter that's where actor_code = x and method = b

This will be a common query since the method number depends on the actor code. i.e. method numbers only have meaning when compared with the actor they were called on.

frrist · 2022-08-09T20:54:07Z

@kasteph, I addressed your feedback, can you give this another look and a ✅ if it looks okay?

kasteph · 2022-08-10T08:29:58Z

@frrist looks good, approved and thank you!

frrist added 2 commits August 2, 2022 14:09

feat: implement vm message extraction

1f883dc

test: update tests and linting

db929fe

frrist force-pushed the frrist/vm-messages branch from 2bf5690 to db929fe Compare August 3, 2022 01:49

frrist linked an issue Aug 3, 2022 that may be closed by this pull request

Full internal message extraction #978

Closed

2 tasks

frrist marked this pull request as ready for review August 3, 2022 02:10

refactor: vm_messages schema indicies; add table comments

3436263

frrist requested review from placer14, davidgasquez and kasteph August 3, 2022 18:15

frrist self-assigned this Aug 3, 2022

frrist commented Aug 3, 2022

View reviewed changes

frrist mentioned this pull request Aug 3, 2022

Full internal message extraction #978

Closed

2 tasks

placer14 approved these changes Aug 4, 2022

View reviewed changes

frrist added 4 commits August 5, 2022 13:03

fix: MarketDealProposal description

e29c595

fix: VmMessage description

59f8217

fix: VmMessage go-pg struct tags

6e88926

- use notnull

fix: index using btree on vmmessage height

ac53c8c

kasteph suggested changes Aug 8, 2022

View reviewed changes

fix: remove redundant indices as they are already PK's

b6b937f

kasteph approved these changes Aug 10, 2022

View reviewed changes

fix: trace processor name

9290f14

frrist merged commit fb9bc26 into master Aug 10, 2022

frrist deleted the frrist/vm-messages branch August 10, 2022 19:41

frrist mentioned this pull request Aug 16, 2022

Story: Lily should track Fault and Termination fees #801

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement vm message extraction #1027

feat: implement vm message extraction #1027

frrist commented Aug 2, 2022 •

edited

Loading

codecov-commenter commented Aug 3, 2022 •

edited

Loading

frrist Aug 3, 2022

placer14 left a comment

placer14 Aug 4, 2022

placer14 Aug 4, 2022

placer14 Aug 4, 2022

frrist Aug 4, 2022 •

edited

Loading

placer14 Aug 5, 2022

frrist Aug 5, 2022 •

edited

Loading

placer14 Aug 5, 2022

placer14 Aug 5, 2022

placer14 Aug 5, 2022

kasteph Aug 8, 2022

placer14 Aug 4, 2022

placer14 Aug 4, 2022

placer14 Aug 4, 2022

placer14 Aug 4, 2022

frrist Aug 4, 2022

placer14 Aug 4, 2022

frrist Aug 4, 2022

placer14 Aug 5, 2022

kasteph Aug 8, 2022

frrist Aug 9, 2022

frrist commented Aug 9, 2022

kasteph commented Aug 10, 2022

	"IsString": "Related to FIP: https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0027.md",
	"IsString": "Indicates if the value of Label is a string. Assume binary blob otherwise. Related to FIP: https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0027.md",

	"ActorCode": "ActorCode of To (receiver)",
	"ActorCode": "ActorCode of To (receiver).",

	Source string `pg:",pk,notnull"`
	Source string `pg:",notnull"`

	Method uint64 `pg:",use_zero"`
	Method uint64 `pg:",notnull,use_zero"`

	ExitCode int64 `pg:",use_zero"`
	ExitCode int64 `pg:",notnull,use_zero"`

	GasUsed int64 `pg:",use_zero"`
	GasUsed int64 `pg:",notnull,use_zero"`

	CREATE INDEX vm_messages_height_idx ON {{ .SchemaName \| default "public"}}.vm_messages USING BRIN (height);
	CREATE INDEX vm_messages_height_idx ON {{ .SchemaName \| default "public"}}.vm_messages USING BTREE (height);

feat: implement vm message extraction #1027

feat: implement vm message extraction #1027

Conversation

frrist commented Aug 2, 2022 • edited Loading

What

codecov-commenter commented Aug 3, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

placer14 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

frrist Aug 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frrist Aug 5, 2022 • 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

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

frrist commented Aug 9, 2022

kasteph commented Aug 10, 2022

frrist commented Aug 2, 2022 •

edited

Loading

codecov-commenter commented Aug 3, 2022 •

edited

Loading

frrist Aug 4, 2022 •

edited

Loading

frrist Aug 5, 2022 •

edited

Loading