Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add new task for fevm trace #1217

Merged
merged 8 commits into from
Jun 7, 2023

Conversation

Terryhung
Copy link
Collaborator

No description provided.

@Terryhung Terryhung marked this pull request as ready for review June 2, 2023 09:01
ActorCode: toActorCode,
Method: uint64(child.Message.Method),
Index: child.Index,
Params: base64.StdEncoding.EncodeToString(child.Message.Params),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referring to the params value returned by lily chain getmessage, we also encode params value in Base64.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the benefit to encode in base64?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the one of reasons for encoding params into base64 is to ensure compatibility: Base64 encoding is a widely supported format, it is able to to mitigate risks across different operating systems and databases.

@Terryhung Terryhung requested a review from birdychang June 2, 2023 16:16
tasks/fevm/vm/task.go Outdated Show resolved Hide resolved
model/fevm/vm.go Outdated Show resolved Hide resolved
ActorCode: toActorCode,
Method: uint64(child.Message.Method),
Index: child.Index,
Params: base64.StdEncoding.EncodeToString(child.Message.Params),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the benefit to encode in base64?

@Terryhung Terryhung requested a review from birdychang June 5, 2023 06:55
message_state_root TEXT,
transaction_hash TEXT,
message_cid TEXT,
trace_cid TEXT,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix indent

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@@ -0,0 +1,199 @@
package fevmvm
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

package name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

tasks "github.com/filecoin-project/lily/tasks"
)

var log = logging.Logger("lily/tasks/fevmvm")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fevmvm?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment on lines 168 to 169
Params: base64.StdEncoding.EncodeToString(child.Message.Params),
Returns: base64.StdEncoding.EncodeToString(child.Receipt.Return),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use EthBytes to be consistent with eth?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

@Terryhung Terryhung changed the title feat: add new task for fevm vm message feat: add new task for fevm trace Jun 7, 2023
GasUsed: 0,
ExitCode: int64(child.Receipt.ExitCode),
ActorCode: toActorCode,
Method: uint64(child.Message.Method),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try to store readable method name

FromEthAddress: fromEthAddress,
ToEthAddress: toEthAddress,
Value: child.Message.Value.String(),
GasUsed: 0,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove gas_used

@Terryhung Terryhung force-pushed the terryhung/add-new-fevm-task-internal-transaction branch from 1b35296 to 33e7571 Compare June 7, 2023 06:52
@Terryhung Terryhung merged commit b907a50 into master Jun 7, 2023
@Terryhung Terryhung deleted the terryhung/add-new-fevm-task-internal-transaction branch June 7, 2023 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants