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

Policy/GAS/Designation contracts RPC wrappers #2643

Merged
merged 4 commits into from
Aug 16, 2022

Conversation

roman-khimov
Copy link
Member

Moving on with #2597.

@roman-khimov roman-khimov added the rpc RPC server and client label Aug 15, 2022
@roman-khimov roman-khimov added this to the v0.99.3 milestone Aug 15, 2022
@roman-khimov roman-khimov force-pushed the policy-contract-rpc-wrapper branch from ffeb5ea to a77708d Compare August 15, 2022 18:33
@codecov
Copy link

codecov bot commented Aug 15, 2022

Codecov Report

Merging #2643 (a77708d) into master (bc9b5d6) will increase coverage by 0.09%.
The diff coverage is 97.70%.

❗ Current head a77708d differs from pull request most recent head 5d54553. Consider uploading reports for the commit 5d54553 to get more accurate results

@@            Coverage Diff             @@
##           master    #2643      +/-   ##
==========================================
+ Coverage   84.83%   84.93%   +0.09%     
==========================================
  Files         306      309       +3     
  Lines       38404    38490      +86     
==========================================
+ Hits        32580    32690     +110     
+ Misses       4450     4421      -29     
- Partials     1374     1379       +5     
Impacted Files Coverage Δ
pkg/rpcclient/native.go 56.17% <ø> (ø)
pkg/rpcclient/policy.go 65.38% <ø> (ø)
cli/wallet/nep17.go 71.26% <50.00%> (-0.36%) ⬇️
pkg/core/interop/context.go 92.03% <100.00%> (ø)
pkg/core/state/contract.go 84.50% <100.00%> (+0.44%) ⬆️
pkg/rpcclient/gas/gas.go 100.00% <100.00%> (ø)
pkg/rpcclient/policy/policy.go 100.00% <100.00%> (ø)
pkg/rpcclient/rolemgmt/roles.go 100.00% <100.00%> (ø)
pkg/network/message.go 94.02% <0.00%> (-3.74%) ⬇️
pkg/core/transaction/transaction.go 85.18% <0.00%> (-1.49%) ⬇️
... and 8 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

GAS itself only has standard NEP-17 methods, so this package only contains its
hash and allows to create NEP-17 structures in an easier way.
*/
package gas
Copy link
Member Author

Choose a reason for hiding this comment

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

BTW, I'm also thinking about gastoken and neotoken.

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 OK with just gas and neo, because we already have an external interop API with gas and neo packages. With it our native interop wrappers and RPC client interface look unified. Also, gas and neo are shorter, which is nice IMO.

Copy link
Member

Choose a reason for hiding this comment

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

Do you want to rename them because of such situations?

gasprom := gas.New(act)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. We use gas a lot for variable names. But some symmetry with interops is also a valid argument.

Copy link
Member

Choose a reason for hiding this comment

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

We may leave it to the user side, in case if he needed he can always use an import alias. But generally, I'm not against renaming.

And test it with the RPC server.

Notice that getters still return int64 instead of *big.Int, that's because
these values are very limited and technically could even fit into an int (but
that seems to be too dangerous to use for long-term compatibility).
@roman-khimov roman-khimov force-pushed the policy-contract-rpc-wrapper branch from a77708d to 5d54553 Compare August 16, 2022 09:43
@roman-khimov roman-khimov merged commit f8857c5 into master Aug 16, 2022
@roman-khimov roman-khimov deleted the policy-contract-rpc-wrapper branch August 16, 2022 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rpc RPC server and client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants