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

cli: implement NEP5-related commands #728

Merged
merged 10 commits into from
Mar 10, 2020
Merged

cli: implement NEP5-related commands #728

merged 10 commits into from
Mar 10, 2020

Conversation

fyrchik
Copy link
Contributor

@fyrchik fyrchik commented Mar 5, 2020

  1. Query balance.
  2. Transfer funds.

Closes #709 .

@codecov
Copy link

codecov bot commented Mar 5, 2020

Codecov Report

Merging #728 into master will decrease coverage by 0.37%.
The diff coverage is 27.58%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #728      +/-   ##
=========================================
- Coverage   66.68%   66.3%   -0.38%     
=========================================
  Files         136     138       +2     
  Lines       12269   12381     +112     
=========================================
+ Hits         8181    8209      +28     
- Misses       3692    3775      +83     
- Partials      396     397       +1
Impacted Files Coverage Δ
pkg/rpc/client/nep5.go 0% <0%> (ø)
pkg/wallet/wallet.go 76.11% <0%> (-2.35%) ⬇️
pkg/util/fixed8.go 100% <100%> (ø) ⬆️
pkg/wallet/token.go 91.66% <91.66%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f1e8f6...c292479. Read the comment docs.

cli/wallet/nep5.go Outdated Show resolved Hide resolved
cli/wallet/nep5.go Outdated Show resolved Hide resolved
pkg/core/state/nep5_test.go Outdated Show resolved Hide resolved
func (t *Token) MarshalJSON() ([]byte, error) {
m := &tokenAux{
Name: t.Name,
Hash: t.Hash.Reverse(), // address should be marshaled in LE but here it is marshaled in LE
Copy link
Member

Choose a reason for hiding this comment

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

What?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tokenAux will be marshalled with the default JSON marshaler, which uses BE.
Token address is used as LE everywhere so it is better to store it the same way.

pkg/wallet/wallet.go Outdated Show resolved Hide resolved
@@ -28,7 +28,7 @@ type Wallet struct {

// Extra metadata can be used for storing arbitrary data.
// This field can be empty.
Extra interface{} `json:"extra"`
Extra Extra `json:"extra"`
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about this. It obviously is more effective storing these values, but at the same time it makes us update the wallet for every NEP5 token and may affect interoperability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is fully compliant with NEP-6 standard.

cli/wallet/nep5.go Outdated Show resolved Hide resolved
cli/wallet/nep5.go Outdated Show resolved Hide resolved
It's size is used in NEP5TransferLog so we need to be
sure it reflects reality.
When working with NEP5 contracts we frequently need
to parse fixed-point decimals with arbitrary precision.
It can be useful to receive all NEP5 token info at once.
To work with NEP5 tokens, one need to save some info
e.g. token Name. This way we will not make additional
RPC just to get Decimals.
getclaimable RPC expects address as a 1-st argument.
@roman-khimov roman-khimov merged commit aab7dd5 into master Mar 10, 2020
@roman-khimov roman-khimov deleted the feature/nep5cli branch March 10, 2020 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Completely new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI command for NEP5 transfers
2 participants