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.

Sign up for GitHub

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

Jump to bottom

Update System.Contract.Call #1647

Merged
merged 3 commits into from
Jan 15, 2021
Merged

Update System.Contract.Call #1647

merged 3 commits into from
Jan 15, 2021

Conversation

fyrchik
Copy link
Contributor

@fyrchik fyrchik commented Dec 29, 2020 •
edited
Loading

Close #1646 .

@codecov
Copy link

codecov bot commented Dec 30, 2020 •
edited
Loading

Codecov Report

Merging #1647 (1c0c331) into master (36b5751) will increase coverage by 0.08%.
The diff coverage is 98.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1647      +/-   ##
==========================================
+ Coverage   82.49%   82.58%   +0.08%     
==========================================
  Files         242      238       -4     
  Lines       19563    19502      -61     
==========================================
- Hits        16139    16106      -33     
+ Misses       2412     2392      -20     
+ Partials     1012     1004       -8
Impacted Files Coverage Δ
pkg/core/interops.go 100.00% <ø> (ø)
pkg/interop/contract/contract.go 0.00% <ø> (ø)
pkg/smartcontract/callflag/call_flags.go 100.00% <ø> (ø)
pkg/vm/interop.go 90.83% <ø> (ø)
pkg/vm/vm.go 93.87% <83.33%> (-0.02%) ⬇️
cli/smartcontract/smart_contract.go 88.23% <100.00%> (+0.04%) ⬆️
cli/wallet/validator.go 71.87% <100.00%> (ø)
pkg/compiler/codegen.go 90.53% <100.00%> (-0.03%) ⬇️
pkg/core/blockchain.go 76.45% <100.00%> (+0.74%) ⬆️
pkg/core/interop/context.go 94.44% <100.00%> (-1.86%) ⬇️
... and 25 more

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 36b5751...1c0c331. Read the comment docs.

@fyrchik
Copy link
Contributor Author

fyrchik commented Dec 30, 2020

There is a problem with RPC: we need to know if invoked method will return any value. Right now we always assume "yes".

AnnaShaleva
AnnaShaleva previously requested changes Jan 12, 2021
View reviewed changes
pkg/core/interop/contract/call.go Outdated Show resolved Hide resolved
@AnnaShaleva
Copy link
Member

There is a problem with RPC: we need to know if invoked method will return any value. Right now we always assume "yes".

We may add one more parameter to the RPC call, can we create an issue for that in C# repo?

This was referenced Jan 12, 2021
Closed
Closed
@roman-khimov
Copy link
Member

There is a problem with RPC: we need to know if invoked method will return any value. Right now we always assume "yes".

Do you mean CreateFunctionInvocationScript? It's not hard to fix it, other than that I probably don't see any problematic cases, when we call some specific methods from client we do know what to expect from them and server-side we just return the stack from test invocations as is whether it has something or not.

But see neo-project/neo-node#710 also.

@fyrchik
Copy link
Contributor Author

fyrchik commented Jan 13, 2021

RPC problem is no longer an issue, after neo-project/neo#2211 .

roman-khimov
roman-khimov requested changes Jan 13, 2021
View reviewed changes
cli/smartcontract/smart_contract.go Outdated Show resolved Hide resolved
pkg/smartcontract/flags/call_flags_test.go Outdated Show resolved Hide resolved
cli/testdata/deploy/main.go Outdated Show resolved Hide resolved
pkg/compiler/syscall.go Outdated Show resolved Hide resolved
pkg/core/interops.go Outdated Show resolved Hide resolved
pkg/vm/emit/emit.go Outdated Show resolved Hide resolved
pkg/vm/emit/emit.go Outdated Show resolved Hide resolved
@fyrchik
smartcontract: move flags to a separate package
dbe81f9
@fyrchik
core: remove callback interops
86b0e76 
Follow neo-project/neo#2168 .
@fyrchik
core: update System.Contract.Call syscall
1c0c331 
1. Remove `System.Contract.CallEx`.
2. Extend number of parameters.
3. Add return value count to `VM.Context`.
@roman-khimov roman-khimov merged commit e36e71f into master Jan 15, 2021
@roman-khimov roman-khimov deleted the contractcall branch January 15, 2021 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roman-khimov roman-khimov roman-khimov approved these changes

@AnnaShaleva AnnaShaleva Awaiting requested review from AnnaShaleva AnnaShaleva is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update System.Contract.Call
3 participants
@fyrchik @AnnaShaleva @roman-khimov