Skip to content

Conversation

containerman17
Copy link
Contributor

A clean version of #1301

Generate a JSON representation of all VM actions and their fields.

This includes all commits from #1198, so it should be merged after that.

When reviewing the code, please compare it with the code in PR 1198, not with the main branch.

@containerman17 containerman17 enabled auto-merge (squash) September 12, 2024 19:18
abi/README.md Outdated
- Built-in types include Address and Bytes.

## Code Generation
Use cmd/abigen to automatically generate Go structs from JSON. No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add instructions instead of saying that this is handled by cmd/abigen ?

abi/README.md Outdated
- JSON files should align with their corresponding .hex files.

## Hash verification
Wallets use ABI to display proper action names and field names. To verify ABI implementation in other languages, marshal the ABI into binary, hash it, and compare with the known hash.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you explain why?

abi/README.md Outdated
Comment on lines 36 to 39
To create an implementation of this package in any other langauge:
- Copy the testdata folder.
- Ensure all marshaling is identical to the Go implementation.
- JSON files should align with their corresponding .hex files.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add a little more detail here. Could we explain the structure of the testdata folder and how an implementation should use it to ensure compliance?

@containerman17 containerman17 merged commit 4778979 into main Sep 12, 2024
16 checks passed
@joshua-kim joshua-kim deleted the abi branch September 13, 2024 02:12
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.

3 participants