-
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
Forward port from 2.x #1188
Forward port from 2.x #1188
Conversation
pkg/core/interop_neo.go
Outdated
@@ -138,6 +138,8 @@ func contractUpdate(ic *interop.Context, v *vm.VM) error { | |||
if len(siMap) != 0 { | |||
return errors.New("old contract shouldn't have storage") | |||
} | |||
} else if !newcontract.ScriptHash().Equals(contract.ScriptHash()) { |
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.
I'm now thinking about the necessity of this check. We have the same one just 20 lines above in case if the script of the new contract isn't' nil, and it returns an error if so. It's technically possible that new script is nil, but shouldn't we return an error in this case (although C# does not)? It seemed to me strange that we can process contracts without script.
Or if the original C# idea was to provide either [new script] or [new manifest] or [new script and manifest] and update only provided values, we don't have matching behavior, because createContractStateFromVM
assumes that manifest is required (and C# do not, they have two checks for null: for script and manifest, see https://github.com/neo-project/neo/blob/master/src/neo/SmartContract/ApplicationEngine.Contract.cs#L78).
So, could you, please, clarify the way this method should work?
Allow to get address from both representations.
d93b703
to
0723571
Compare
Fixes #1144. It's quite simple approach, we just update balance info right upon contract migration. It will slow down migration transactions, but it takes about 1-2 seconds to Seek through balances at mainnet's 3.8M, so the approach should still work good enough. The other idea was to make lazy updates (maintaining contract migration map), but it's more complicated to implement (and implies that a balance get might also do a write). There also is a concern about memory usage, it can give a spike of some tens of megabytes, but that also is considered to be acceptable.
We can't lock them (or there will be a deadlock), but we need to fix this: fatal error: concurrent map iteration and map write goroutine 1 [running]: runtime.throw(0xdec086, 0x26) /usr/lib64/go/1.12/src/runtime/panic.go:617 +0x72 fp=0xc02fec2bf8 sp=0xc02fec2bc8 pc=0x42d932 runtime.mapiternext(0xc02fec2d40) /usr/lib64/go/1.12/src/runtime/map.go:860 +0x597 fp=0xc02fec2c80 sp=0xc02fec2bf8 pc=0x40efe7 github.com/nspcc-dev/neo-go/pkg/network.(*Server).Shutdown(0xc0000fc160) /home/rik/dev/neo-go2/pkg/network/server.go:194 +0x238 fp=0xc02fec2db0 sp=0xc02fec2c80 pc=0xa89da8 github.com/nspcc-dev/neo-go/cli/server.startServer(0xc0000fcc60, 0x0, 0x0) /home/rik/dev/neo-go2/cli/server/server.go:399 +0x7a9 fp=0xc02fec3820 sp=0xc02fec2db0 pc=0xae2079 ...
0723571
to
432cef5
Compare
Codecov Report
@@ Coverage Diff @@
## master #1188 +/- ##
==========================================
- Coverage 67.00% 66.92% -0.09%
==========================================
Files 199 199
Lines 16919 16971 +52
==========================================
+ Hits 11336 11357 +21
- Misses 4978 5005 +27
- Partials 605 609 +4
Continue to review full report at Codecov.
|
Problem
2.x branch has accumulated some useful changes.
Solution
Forward-port them to master.