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(amino): Add TypeDesc Amino implementation and TypedValue marshaler #2113

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

gfanton
Copy link
Member

@gfanton gfanton commented May 15, 2024

PS: This PR is still in draft, but I would welcome any feedback on it

This PR has two main objectives. The first is to add a TypeDesc method implementation to Amino, allowing a type to self-describe. The second is to introduce a new package, gnoamino, dedicated to (un)marshaling Gno types, starting with gnolang.TypedValue and its TypedValueWrapper, which implements the TypeDesc Amino method.

Most of the methods in TypedValueWrapper are inspired by or copied from gnonative.go. They are exported here for several reasons:

  1. The methods in gnonative are intended for general use and may be removed later, while the ones here are focused solely on marshaling and unmarshaling.
  2. These methods can be simpler, as they do not require the full support of reflection, enabling a more detailed implementation.
  3. We assume prior knowledge of the type of the TypedValue, simplifying its management.
  4. We will likely implement gas consumption calculations within those methods.
Contributors' checklist...
  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com>
Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com>
Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com>
Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com>
Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com>
@github-actions github-actions bot added 📦 🤖 gnovm Issues or PRs gnovm related 📦 🌐 tendermint v2 Issues or PRs tm2 related labels May 15, 2024
Copy link

codecov bot commented May 15, 2024

Codecov Report

Attention: Patch coverage is 33.87097% with 41 lines in your changes are missing coverage. Please review.

Project coverage is 49.07%. Comparing base (ccc6d5b) to head (9dd01a1).

Files Patch % Lines
tm2/pkg/amino/json_decode.go 50.00% 7 Missing and 5 partials ⚠️
tm2/pkg/amino/reflect.go 0.00% 8 Missing ⚠️
tm2/pkg/amino/codec.go 14.28% 5 Missing and 1 partial ⚠️
tm2/pkg/amino/binary_decode.go 37.50% 3 Missing and 2 partials ⚠️
tm2/pkg/amino/binary_encode.go 28.57% 3 Missing and 2 partials ⚠️
tm2/pkg/amino/json_encode.go 37.50% 3 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2113      +/-   ##
==========================================
+ Coverage   49.04%   49.07%   +0.03%     
==========================================
  Files         576      578       +2     
  Lines       77556    77937     +381     
==========================================
+ Hits        38035    38250     +215     
- Misses      36439    36581     +142     
- Partials     3082     3106      +24     
Flag Coverage Δ
tm2 54.54% <33.87%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com>
Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com>
Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com>
Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com>
Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

1 participant