-
Notifications
You must be signed in to change notification settings - Fork 79
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
native: fix unclaimedGas calculation #3594
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3594 +/- ##
=======================================
Coverage 85.20% 85.20%
=======================================
Files 333 333
Lines 39005 39007 +2
=======================================
+ Hits 33233 33235 +2
- Misses 4203 4204 +1
+ Partials 1569 1568 -1 ☔ View full report in Codecov by Sentry. |
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 need tests for it. IIUC the problem was just for zero (or nonexistent) balance? Regular balances behave differently (execution breaks), it's just a matter of check order?
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 fix itself is legit, please add results for the T5 test with non-zero Neo balance and also please, add unit-tests to cover uncovered paths in neo-go/pkg/core/native/native_test/neo_test.go
.
Yes, it is. |
I'd say that our old code was a bit better since it checked inputs first and only touched the DB afterwards. But it's not worth fixing on the C# side at the same time (introducing HF-dependent behaviour). |
94330ae
to
401b2e8
Compare
@AliceInHunterland, have you managed to invoke this method on T5 testnet for account with non-zero balance? Please, attach invocation results for both C# and Go nodes to the PR, for both cases when requested height is valid and invalid. |
|
|
@AliceInHunterland, we need results for both C# and Go nodes for the same transaction. For 2 transaction is not even sent, use Also, reviewcomments are not fixed. |
401b2e8
to
e12024a
Compare
Fix difference with C#: ``` (base) ekaterinapavlova@MacBook-Air-4 neo-go % curl -X POST http://seed1t5.neo.org:20332 -H 'Content-Type: application/json' -d '{ "jsonrpc": "2.0", "method": "getapplicationlog", "params": ["61681ce24dffea5481e9a50b10159b43b7ebfc21967b0b06fee7ff69c7123e3f"], "id": 1 }' | json_pp % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 429 0 269 100 160 582 346 --:--:-- --:--:-- --:--:-- 930 { "id" : 1, "jsonrpc" : "2.0", "result" : { "executions" : [ { "exception" : null, "gasconsumed" : "198754", "notifications" : [], "stack" : [ { "type" : "Integer", "value" : "0" } ], "trigger" : "Application", "vmstate" : "HALT" } ], "txid" : "0x61681ce24dffea5481e9a50b10159b43b7ebfc21967b0b06fee7ff69c7123e3 f" } } ``` (base) ekaterinapavlova@MacBook-Air-4 neo-go % curl -X POST https://rpc .t5.n3.nspcc.ru:20331 -H 'Content-Type: application/json' -d '{ "jsonrpc": "2.0", "method": "getapplicationlog", "params": ["61681ce24dffea5481e9a50b10159b43b7ebfc21967b0b06fee7ff69c7123e3f"], "id": 1 }' | json_pp % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 583 100 423 100 160 1424 538 --:--:-- --:--:-- --:--:-- 1969 { "id" : 1, "jsonrpc" : "2.0", "result" : { "executions" : [ { "exception" : "at instruction 120 (SYSCALL): can't calculate bonus of height unequal (BlockHeight + 1)", "gasconsumed" : "198754", "notifications" : [], "stack" : [ { "type" : "Integer", "value" : "4704605" }, { "type" : "ByteString", "value" : "KfYYlDe/fxqqqm1yr7o5XLnQ7uk=" } ], "trigger" : "Application", "vmstate" : "FAULT" } ], "txid" : "0x61681ce24dffea5481e9a50b10159b43b7ebfc21967b0b06fee7ff69c7123e3 f" } } ``` ``` (base) ekaterinapavlova@MacBook-Air-4 neo-go % ./bin/neo-go contract invokefunction -r https://rpc.t5.n3.nspcc.ru:20331 -w ./testnet_wallet .json 1e6f88377a6c6bc4f683a5fc61eed5645ec5f123 unclaimedGas e9eed0b95c39baaf726daaaa1a7fbf379418f629 4704605 Enter account NWtk9HYWsf1njtSzA3XNgwZXRtriACcJ9G password > Warning: FAULT VM state returned from the RPC node: at instruction 120 (SYSCALL): can't calculate bonus of height unequal (BlockHeight + 1). Use --force flag to send the transaction anyway. ``` Close #3589 Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
e12024a
to
02727b1
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.
Good to go, we need 0.106.4...
Fix difference with C#:
Close #3589