native: fix contract hashes #1616
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1616 +/- ##
==========================================
+ Coverage 82.35% 82.54% +0.18%
==========================================
Files 241 241
Lines 19022 19018 -4
==========================================
+ Hits 15666 15698 +32
+ Misses 2366 2335 -31
+ Partials 990 985 -5
Continue to review full report at Codecov.
|
pkg/core/blockchain.go
Outdated
| @@ -1657,12 +1657,12 @@ var ( | |||
| func (bc *Blockchain) initVerificationVM(ic *interop.Context, hash util.Uint160, witness *transaction.Witness) error { | |||
| v := ic.VM | |||
| if len(witness.VerificationScript) != 0 { | |||
| if witness.ScriptHash() != hash { | |||
| return ErrWitnessHashMismatch | |||
| } | |||
There was a problem hiding this comment.
Is this change really needed?
There was a problem hiding this comment.
No, this was only to get a specific error in test. I think we can remove the second check now, while leaving the test.
There was a problem hiding this comment.
I don't see any test being changed, so we either need that change or don't need this one.
pkg/core/interop/context.go
Outdated
| @@ -113,7 +114,7 @@ func NewContractMD(name string) *ContractMD { | |||
| emit.Syscall(w.BinWriter, interopnames.NeoNativeCall) | |||
|
|
|||
| c.Script = w.Bytes() | |||
| c.Hash = hash.Hash160(c.Script) | |||
| c.Hash = state.CreateContractHash(hash.Hash160([]byte{byte(opcode.PUSH1)}), c.Script) | |||
There was a problem hiding this comment.
We better merge #1610 now first and then this one. And add some test that specifically checks for native contract's hashes and compares them to known values from C# devpack (see neo-project/neo-devpack-dotnet#391).
There was a problem hiding this comment.
We can also add some method like CreateNativeContractHash(name string) to reduce code duplication between native contracts.
There was a problem hiding this comment.
Took it directly from C# node, devpack has outdated values. Fixed.
pkg/core/interop/context.go
Outdated
| @@ -119,7 +118,7 @@ func NewContractMD(name string) *ContractMD { | |||
| emit.Syscall(w.BinWriter, interopnames.SystemContractCallNative) | |||
|
|
|||
| c.Script = w.Bytes() | |||
| c.Hash = hash.Hash160(c.Script) | |||
| c.Hash = state.CreateNativeContractHash(c.Script) | |||
There was a problem hiding this comment.
CreateNativeContractHash(name string), move all these binwriter things into the function.
pkg/core/blockchain.go
Outdated
| @@ -1657,12 +1657,12 @@ var ( | |||
| func (bc *Blockchain) initVerificationVM(ic *interop.Context, hash util.Uint160, witness *transaction.Witness) error { | |||
| v := ic.VM | |||
| if len(witness.VerificationScript) != 0 { | |||
| if witness.ScriptHash() != hash { | |||
| return ErrWitnessHashMismatch | |||
| } | |||
There was a problem hiding this comment.
I don't see any test being changed, so we either need that change or don't need this one.
They use fictive sender now.