-
Notifications
You must be signed in to change notification settings - Fork 2
Dynamic msg TL-B - JSON decoding + chainlink-ton-extras pkg #269
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
Conversation
|
👋 krebernisak, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
644965b to
c5046f5
Compare
7fdfd97 to
c4530b5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work you did here. The only mistake I noticed is what Nico already pointed out about the error message.
c90ee6b to
088f197
Compare
8a1cf4d to
e2de883
Compare
|
|
||
| type decoder struct{} | ||
|
|
||
| func NewDecoder() lib.ContractDecoder { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll rethink the lib.ContractDecoder as the decoding becomes more dynamic, for now keeping it in this PR to match existing infra.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lib.ContractDecoder implementations are now simplified and made dynamic from bindings TLB types and error code definitions. It's now a thin layer that can potentially be removed in a followup.
I've left it in this PR as the explorer util depends on it - to not introduce more breaking changes.
There's still some custom event decoding left in specific lib.ContractDecoder implementations, but this can also be generalized and removed in a followup.
11293f4 to
ef3d01a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks awesome, I wonder if we'll need a diff check for generated files
ef3d01a to
b47c607
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like some documentation on how to update the generated code, and we need to get rid of the binary. Otherwise, looks good! Explorer decoding is working again 🚀
2d29e83 to
1a32c9a
Compare
944521d to
261a1ca
Compare
af81a82 to
3c7fe5b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR simplifies and makes TL-B - JSON decoding dynamic, and adds a new
chainlink-ton-extrasNix pkg to host theexplorerbin and other extras (tools) we'd want to package.Try this branch directly: