From b542a5e7a0dc88b0f751818530c5be1686cc6a9a Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 9 Dec 2019 16:57:25 +0300 Subject: [PATCH 1/5] io: add support for pointer receivers in WriteArray() It's actually preferable to have pointer receivers for serializable types, so this should be supported. --- pkg/io/binaryWriter.go | 5 ++++- pkg/io/binaryrw_test.go | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/pkg/io/binaryWriter.go b/pkg/io/binaryWriter.go index 5b6ccade9..d9ea336e2 100644 --- a/pkg/io/binaryWriter.go +++ b/pkg/io/binaryWriter.go @@ -49,7 +49,10 @@ func (w *BinWriter) WriteArray(arr interface{}) { for i := 0; i < val.Len(); i++ { el, ok := val.Index(i).Interface().(encodable) if !ok { - panic(typ.String() + "is not encodable") + el, ok = val.Index(i).Addr().Interface().(encodable) + if !ok { + panic(typ.String() + " is not encodable") + } } el.EncodeBinary(w) diff --git a/pkg/io/binaryrw_test.go b/pkg/io/binaryrw_test.go index 40a6d42f2..a3e55c0eb 100644 --- a/pkg/io/binaryrw_test.go +++ b/pkg/io/binaryrw_test.go @@ -229,6 +229,18 @@ func (t *testSerializable) DecodeBinary(r *BinReader) { r.ReadLE(t) } +type testPtrSerializable uint16 + +// EncodeBinary implements io.Serializable interface. +func (t *testPtrSerializable) EncodeBinary(w *BinWriter) { + w.WriteLE(t) +} + +// DecodeBinary implements io.Serializable interface. +func (t *testPtrSerializable) DecodeBinary(r *BinReader) { + r.ReadLE(t) +} + func TestBinWriter_WriteArray(t *testing.T) { var arr [3]testSerializable for i := range arr { @@ -272,6 +284,16 @@ func TestBinWriter_WriteArray(t *testing.T) { w.Reset() w.Err = errors.New("error") require.Panics(t, func() { w.WriteArray(make(chan testSerializable)) }) + + // Ptr receiver test + var arrPtr [3]testPtrSerializable + for i := range arrPtr { + arrPtr[i] = testPtrSerializable(i) + } + w.Reset() + w.WriteArray(arr[:]) + require.NoError(t, w.Err) + require.Equal(t, expected, w.Bytes()) } func TestBinReader_ReadArray(t *testing.T) { From 7e371588a7186a13786abb246395b3f52d066873 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 9 Dec 2019 17:14:10 +0300 Subject: [PATCH 2/5] core/tx: remove one layer of indirection for scripts and inouts It reduces heap pressure a little for these elements as we don't have to allocate/free them individually. And they're directly tied to transactions or block, not being shared or anything like that, so it makes little sense for them to be pointer-based. It only makes building transactions a little easier, but that's obviously a minor usecase. --- pkg/consensus/consensus.go | 2 +- pkg/consensus/payload_test.go | 8 ++++---- pkg/core/block.go | 1 - pkg/core/block_base.go | 3 +-- pkg/core/block_test.go | 2 +- pkg/core/blockchain.go | 13 ++++++------ pkg/core/header_test.go | 2 +- pkg/core/helper_test.go | 2 +- pkg/core/interop_neo.go | 18 ++++++++-------- pkg/core/interop_neo_test.go | 32 ++++++++++++++--------------- pkg/core/mem_pool.go | 4 +++- pkg/core/transaction/invocation.go | 8 ++++---- pkg/core/transaction/transaction.go | 22 +++++++++++--------- pkg/core/util.go | 32 ++++++++++++++--------------- pkg/network/payload/headers_test.go | 6 +++--- pkg/rpc/txBuilder.go | 4 ++-- 16 files changed, 81 insertions(+), 78 deletions(-) diff --git a/pkg/consensus/consensus.go b/pkg/consensus/consensus.go index 9aeeeb516..996c30e78 100644 --- a/pkg/consensus/consensus.go +++ b/pkg/consensus/consensus.go @@ -258,7 +258,7 @@ func (s *service) verifyBlock(b block.Block) bool { func (s *service) processBlock(b block.Block) { bb := &b.(*neoBlock).Block - bb.Script = s.getBlockWitness(bb) + bb.Script = *(s.getBlockWitness(bb)) if err := s.Chain.AddBlock(bb); err != nil { s.log.Warnf("error on add block: %v", err) diff --git a/pkg/consensus/payload_test.go b/pkg/consensus/payload_test.go index c6312c006..26c2c4b42 100644 --- a/pkg/consensus/payload_test.go +++ b/pkg/consensus/payload_test.go @@ -339,10 +339,10 @@ func newMinerTx(nonce uint32) *transaction.Transaction { Data: &transaction.MinerTX{ Nonce: rand.Uint32(), }, - Attributes: []*transaction.Attribute{}, - Inputs: []*transaction.Input{}, - Outputs: []*transaction.Output{}, - Scripts: []*transaction.Witness{}, + Attributes: []transaction.Attribute{}, + Inputs: []transaction.Input{}, + Outputs: []transaction.Output{}, + Scripts: []transaction.Witness{}, Trimmed: false, } } diff --git a/pkg/core/block.go b/pkg/core/block.go index ff4266276..e1e898ed7 100644 --- a/pkg/core/block.go +++ b/pkg/core/block.go @@ -91,7 +91,6 @@ func NewBlockFromTrimmedBytes(b []byte) (*Block, error) { var padding uint8 br.ReadLE(&padding) - block.Script = &transaction.Witness{} block.Script.DecodeBinary(br) lenTX := br.ReadVarUint() diff --git a/pkg/core/block_base.go b/pkg/core/block_base.go index 7ca235ae2..77ec7d61d 100644 --- a/pkg/core/block_base.go +++ b/pkg/core/block_base.go @@ -38,7 +38,7 @@ type BlockBase struct { _ uint8 // Script used to validate the block - Script *transaction.Witness `json:"script"` + Script transaction.Witness `json:"script"` // Hash of this block, created when binary encoded (double SHA256). hash util.Uint256 @@ -80,7 +80,6 @@ func (b *BlockBase) DecodeBinary(br *io.BinReader) { return } - b.Script = &transaction.Witness{} b.Script.DecodeBinary(br) } diff --git a/pkg/core/block_test.go b/pkg/core/block_test.go index b39e486c4..594041158 100644 --- a/pkg/core/block_test.go +++ b/pkg/core/block_test.go @@ -87,7 +87,7 @@ func newDumbBlock() *Block { Index: 1, ConsensusData: 1111, NextConsensus: hash.Hash160([]byte("a")), - Script: &transaction.Witness{ + Script: transaction.Witness{ VerificationScript: []byte{0x51}, // PUSH1 InvocationScript: []byte{0x61}, // NOP }, diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 9a12d911c..6544ebc4b 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -945,7 +945,7 @@ func (bc *Blockchain) References(t *transaction.Transaction) map[transaction.Inp tx = nil } else if tx != nil { for _, in := range inputs { - references[*in] = tx.Outputs[in.PrevIndex] + references[*in] = &tx.Outputs[in.PrevIndex] } } else { references = nil @@ -1243,8 +1243,9 @@ func (bc *Blockchain) GetValidators(txes ...*transaction.Transaction) ([]*keys.P // group inputs by the same previous hash and iterate through inputs group := make(map[util.Uint256][]*transaction.Input) - for _, input := range tx.Inputs { - group[input.PrevHash] = append(group[input.PrevHash], input) + for i := range tx.Inputs { + hash := tx.Inputs[i].PrevHash + group[hash] = append(group[hash], &tx.Inputs[i]) } for hash, inputs := range group { @@ -1360,7 +1361,7 @@ func (bc *Blockchain) GetScriptHashesForVerifying(t *transaction.Transaction) ([ } hashes := make(map[util.Uint160]bool) for _, i := range t.Inputs { - h := references[*i].ScriptHash + h := references[i].ScriptHash if _, ok := hashes[h]; !ok { hashes[h] = true } @@ -1486,7 +1487,7 @@ func (bc *Blockchain) verifyTxWitnesses(t *transaction.Transaction, block *Block sort.Slice(witnesses, func(i, j int) bool { return witnesses[i].ScriptHash().Less(witnesses[j].ScriptHash()) }) interopCtx := newInteropContext(trigger.Verification, bc, bc.store, block, t) for i := 0; i < len(hashes); i++ { - err := bc.verifyHashAgainstScript(hashes[i], witnesses[i], t.VerificationHash(), interopCtx) + err := bc.verifyHashAgainstScript(hashes[i], &witnesses[i], t.VerificationHash(), interopCtx) if err != nil { numStr := fmt.Sprintf("witness #%d", i) return errors.Wrap(err, numStr) @@ -1505,7 +1506,7 @@ func (bc *Blockchain) verifyBlockWitnesses(block *Block, prevHeader *Header) err hash = prevHeader.NextConsensus } interopCtx := newInteropContext(trigger.Verification, bc, bc.store, nil, nil) - return bc.verifyHashAgainstScript(hash, block.Script, block.VerificationHash(), interopCtx) + return bc.verifyHashAgainstScript(hash, &block.Script, block.VerificationHash(), interopCtx) } func hashAndIndexToBytes(h util.Uint256, index uint32) []byte { diff --git a/pkg/core/header_test.go b/pkg/core/header_test.go index 81cd2a242..ed35c6f73 100644 --- a/pkg/core/header_test.go +++ b/pkg/core/header_test.go @@ -20,7 +20,7 @@ func TestHeaderEncodeDecode(t *testing.T) { Index: 3445, ConsensusData: 394949, NextConsensus: util.Uint160{}, - Script: &transaction.Witness{ + Script: transaction.Witness{ InvocationScript: []byte{0x10}, VerificationScript: []byte{0x11}, }, diff --git a/pkg/core/helper_test.go b/pkg/core/helper_test.go index 43b8d96ab..132dc5087 100644 --- a/pkg/core/helper_test.go +++ b/pkg/core/helper_test.go @@ -55,7 +55,7 @@ func newBlock(index uint32, txs ...*transaction.Transaction) *Block { vlen-(vlen-1)/3, validators, ) - witness := &transaction.Witness{ + witness := transaction.Witness{ VerificationScript: valScript, } b := &Block{ diff --git a/pkg/core/interop_neo.go b/pkg/core/interop_neo.go index c7bea1c0f..da688b9cc 100644 --- a/pkg/core/interop_neo.go +++ b/pkg/core/interop_neo.go @@ -89,8 +89,8 @@ func (ic *interopContext) txGetAttributes(v *vm.VM) error { return errors.New("too many attributes") } attrs := make([]vm.StackItem, 0, len(tx.Attributes)) - for _, attr := range tx.Attributes { - attrs = append(attrs, vm.NewInteropItem(attr)) + for i := range tx.Attributes { + attrs = append(attrs, vm.NewInteropItem(&tx.Attributes[i])) } v.Estack().PushVal(attrs) return nil @@ -107,8 +107,8 @@ func (ic *interopContext) txGetInputs(v *vm.VM) error { return errors.New("too many inputs") } inputs := make([]vm.StackItem, 0, len(tx.Inputs)) - for _, input := range tx.Inputs { - inputs = append(inputs, vm.NewInteropItem(input)) + for i := range tx.Inputs { + inputs = append(inputs, vm.NewInteropItem(&tx.Inputs[i])) } v.Estack().PushVal(inputs) return nil @@ -125,8 +125,8 @@ func (ic *interopContext) txGetOutputs(v *vm.VM) error { return errors.New("too many outputs") } outputs := make([]vm.StackItem, 0, len(tx.Outputs)) - for _, output := range tx.Outputs { - outputs = append(outputs, vm.NewInteropItem(output)) + for i := range tx.Outputs { + outputs = append(outputs, vm.NewInteropItem(&tx.Outputs[i])) } v.Estack().PushVal(outputs) return nil @@ -146,7 +146,7 @@ func (ic *interopContext) txGetReferences(v *vm.VM) error { stackrefs := make([]vm.StackItem, 0, len(refs)) for _, k := range tx.Inputs { - tio := txInOut{*k, *refs[*k]} + tio := txInOut{k, *refs[k]} stackrefs = append(stackrefs, vm.NewInteropItem(tio)) } v.Estack().PushVal(stackrefs) @@ -190,8 +190,8 @@ func (ic *interopContext) txGetWitnesses(v *vm.VM) error { return errors.New("too many outputs") } scripts := make([]vm.StackItem, 0, len(tx.Scripts)) - for _, script := range tx.Scripts { - scripts = append(scripts, vm.NewInteropItem(script)) + for i := range tx.Scripts { + scripts = append(scripts, vm.NewInteropItem(&tx.Scripts[i])) } v.Estack().PushVal(scripts) return nil diff --git a/pkg/core/interop_neo_test.go b/pkg/core/interop_neo_test.go index 331f593bf..c1c378cd9 100644 --- a/pkg/core/interop_neo_test.go +++ b/pkg/core/interop_neo_test.go @@ -92,7 +92,7 @@ func TestTxGetInputs(t *testing.T) { err := context.txGetInputs(v) require.NoError(t, err) value := v.Estack().Pop().Value().([]vm.StackItem) - require.Equal(t, tx.Inputs[0], value[0].Value().(*transaction.Input)) + require.Equal(t, tx.Inputs[0], *value[0].Value().(*transaction.Input)) } func TestTxGetOutputs(t *testing.T) { @@ -101,7 +101,7 @@ func TestTxGetOutputs(t *testing.T) { err := context.txGetOutputs(v) require.NoError(t, err) value := v.Estack().Pop().Value().([]vm.StackItem) - require.Equal(t, tx.Outputs[0], value[0].Value().(*transaction.Output)) + require.Equal(t, tx.Outputs[0], *value[0].Value().(*transaction.Output)) } func TestTxGetType(t *testing.T) { @@ -115,16 +115,16 @@ func TestTxGetType(t *testing.T) { func TestPopInputFromVM(t *testing.T) { v, tx, _ := createVMAndTX(t) - v.Estack().PushVal(vm.NewInteropItem(tx.Inputs[0])) + v.Estack().PushVal(vm.NewInteropItem(&tx.Inputs[0])) input, err := popInputFromVM(v) require.NoError(t, err) - require.Equal(t, tx.Inputs[0], input) + require.Equal(t, tx.Inputs[0], *input) } func TestInputGetHash(t *testing.T) { v, tx, context := createVMAndTX(t) - v.Estack().PushVal(vm.NewInteropItem(tx.Inputs[0])) + v.Estack().PushVal(vm.NewInteropItem(&tx.Inputs[0])) err := context.inputGetHash(v) require.NoError(t, err) @@ -134,7 +134,7 @@ func TestInputGetHash(t *testing.T) { func TestInputGetIndex(t *testing.T) { v, tx, context := createVMAndTX(t) - v.Estack().PushVal(vm.NewInteropItem(tx.Inputs[0])) + v.Estack().PushVal(vm.NewInteropItem(&tx.Inputs[0])) err := context.inputGetIndex(v) require.NoError(t, err) @@ -144,16 +144,16 @@ func TestInputGetIndex(t *testing.T) { func TestPopOutputFromVM(t *testing.T) { v, tx, _ := createVMAndTX(t) - v.Estack().PushVal(vm.NewInteropItem(tx.Outputs[0])) + v.Estack().PushVal(vm.NewInteropItem(&tx.Outputs[0])) output, err := popOutputFromVM(v) require.NoError(t, err) - require.Equal(t, tx.Outputs[0], output) + require.Equal(t, tx.Outputs[0], *output) } func TestOutputGetAssetID(t *testing.T) { v, tx, context := createVMAndTX(t) - v.Estack().PushVal(vm.NewInteropItem(tx.Outputs[0])) + v.Estack().PushVal(vm.NewInteropItem(&tx.Outputs[0])) err := context.outputGetAssetID(v) require.NoError(t, err) @@ -163,7 +163,7 @@ func TestOutputGetAssetID(t *testing.T) { func TestOutputGetScriptHash(t *testing.T) { v, tx, context := createVMAndTX(t) - v.Estack().PushVal(vm.NewInteropItem(tx.Outputs[0])) + v.Estack().PushVal(vm.NewInteropItem(&tx.Outputs[0])) err := context.outputGetScriptHash(v) require.NoError(t, err) @@ -173,7 +173,7 @@ func TestOutputGetScriptHash(t *testing.T) { func TestOutputGetValue(t *testing.T) { v, tx, context := createVMAndTX(t) - v.Estack().PushVal(vm.NewInteropItem(tx.Outputs[0])) + v.Estack().PushVal(vm.NewInteropItem(&tx.Outputs[0])) err := context.outputGetValue(v) require.NoError(t, err) @@ -183,7 +183,7 @@ func TestOutputGetValue(t *testing.T) { func TestAttrGetData(t *testing.T) { v, tx, context := createVMAndTX(t) - v.Estack().PushVal(vm.NewInteropItem(tx.Attributes[0])) + v.Estack().PushVal(vm.NewInteropItem(&tx.Attributes[0])) err := context.attrGetData(v) require.NoError(t, err) @@ -193,7 +193,7 @@ func TestAttrGetData(t *testing.T) { func TestAttrGetUsage(t *testing.T) { v, tx, context := createVMAndTX(t) - v.Estack().PushVal(vm.NewInteropItem(tx.Attributes[0])) + v.Estack().PushVal(vm.NewInteropItem(&tx.Attributes[0])) err := context.attrGetUsage(v) require.NoError(t, err) @@ -397,17 +397,17 @@ func createVMAndTX(t *testing.T) (*vm.VM, *transaction.Transaction, *interopCont tx := newMinerTX() bytes := make([]byte, 1) - attributes := append(tx.Attributes, &transaction.Attribute{ + attributes := append(tx.Attributes, transaction.Attribute{ Usage: transaction.Description, Data: bytes, }) - inputs := append(tx.Inputs, &transaction.Input{ + inputs := append(tx.Inputs, transaction.Input{ PrevHash: randomUint256(), PrevIndex: 1, }) - outputs := append(tx.Outputs, &transaction.Output{ + outputs := append(tx.Outputs, transaction.Output{ AssetID: randomUint256(), Amount: 10, ScriptHash: randomUint160(), diff --git a/pkg/core/mem_pool.go b/pkg/core/mem_pool.go index 52611dc74..d9e489c0f 100644 --- a/pkg/core/mem_pool.go +++ b/pkg/core/mem_pool.go @@ -288,7 +288,9 @@ func (mp MemPool) Verify(tx *transaction.Transaction) bool { inputs := make([]*transaction.Input, 0) for _, item := range mp.GetVerifiedTransactions() { if tx.Hash().Equals(item.Hash()) { - inputs = append(inputs, item.Inputs...) + for i := range item.Inputs { + inputs = append(inputs, &item.Inputs[i]) + } } } diff --git a/pkg/core/transaction/invocation.go b/pkg/core/transaction/invocation.go index 50ad3c78f..cf60cb403 100644 --- a/pkg/core/transaction/invocation.go +++ b/pkg/core/transaction/invocation.go @@ -26,10 +26,10 @@ func NewInvocationTX(script []byte, gas util.Fixed8) *Transaction { Gas: gas, Version: 1, }, - Attributes: []*Attribute{}, - Inputs: []*Input{}, - Outputs: []*Output{}, - Scripts: []*Witness{}, + Attributes: []Attribute{}, + Inputs: []Input{}, + Outputs: []Output{}, + Scripts: []Witness{}, } } diff --git a/pkg/core/transaction/transaction.go b/pkg/core/transaction/transaction.go index 36443ec3b..305a577d2 100644 --- a/pkg/core/transaction/transaction.go +++ b/pkg/core/transaction/transaction.go @@ -27,18 +27,18 @@ type Transaction struct { Data TXer `json:"-"` // Transaction attributes. - Attributes []*Attribute `json:"attributes"` + Attributes []Attribute `json:"attributes"` // The inputs of the transaction. - Inputs []*Input `json:"vin"` + Inputs []Input `json:"vin"` // The outputs of the transaction. - Outputs []*Output `json:"vout"` + Outputs []Output `json:"vout"` // The scripts that comes with this transaction. // Scripts exist out of the verification script // and invocation script. - Scripts []*Witness `json:"scripts"` + Scripts []Witness `json:"scripts"` // Hash of the transaction (double SHA256). hash util.Uint256 @@ -82,12 +82,12 @@ func (t *Transaction) VerificationHash() util.Uint256 { // AddOutput adds the given output to the transaction outputs. func (t *Transaction) AddOutput(out *Output) { - t.Outputs = append(t.Outputs, out) + t.Outputs = append(t.Outputs, *out) } // AddInput adds the given input to the transaction inputs. func (t *Transaction) AddInput(in *Input) { - t.Inputs = append(t.Inputs, in) + t.Inputs = append(t.Inputs, *in) } // DecodeBinary implements Serializable interface. @@ -187,8 +187,9 @@ func (t *Transaction) createHash() error { // GroupInputsByPrevHash groups all TX inputs by their previous hash. func (t *Transaction) GroupInputsByPrevHash() map[util.Uint256][]*Input { m := make(map[util.Uint256][]*Input) - for _, in := range t.Inputs { - m[in.PrevHash] = append(m[in.PrevHash], in) + for i := range t.Inputs { + hash := t.Inputs[i].PrevHash + m[hash] = append(m[hash], &t.Inputs[i]) } return m } @@ -196,8 +197,9 @@ func (t *Transaction) GroupInputsByPrevHash() map[util.Uint256][]*Input { // GroupOutputByAssetID groups all TX outputs by their assetID. func (t Transaction) GroupOutputByAssetID() map[util.Uint256][]*Output { m := make(map[util.Uint256][]*Output) - for _, out := range t.Outputs { - m[out.AssetID] = append(m[out.AssetID], out) + for i := range t.Outputs { + hash := t.Outputs[i].AssetID + m[hash] = append(m[hash], &t.Outputs[i]) } return m } diff --git a/pkg/core/util.go b/pkg/core/util.go index 05ba10bdd..8da60beab 100644 --- a/pkg/core/util.go +++ b/pkg/core/util.go @@ -31,7 +31,7 @@ func createGenesisBlock(cfg config.ProtocolConfiguration) (*Block, error) { Index: 0, ConsensusData: 2083236893, NextConsensus: nextConsensus, - Script: &transaction.Witness{ + Script: transaction.Witness{ InvocationScript: []byte{}, VerificationScript: []byte{byte(opcode.PUSHT)}, }, @@ -56,25 +56,25 @@ func createGenesisBlock(cfg config.ProtocolConfiguration) (*Block, error) { Data: &transaction.MinerTX{ Nonce: 2083236893, }, - Attributes: []*transaction.Attribute{}, - Inputs: []*transaction.Input{}, - Outputs: []*transaction.Output{}, - Scripts: []*transaction.Witness{}, + Attributes: []transaction.Attribute{}, + Inputs: []transaction.Input{}, + Outputs: []transaction.Output{}, + Scripts: []transaction.Witness{}, }, governingTX, utilityTX, { Type: transaction.IssueType, Data: &transaction.IssueTX{}, // no fields. - Inputs: []*transaction.Input{}, - Outputs: []*transaction.Output{ + Inputs: []transaction.Input{}, + Outputs: []transaction.Output{ { AssetID: governingTX.Hash(), Amount: governingTX.Data.(*transaction.RegisterTX).Amount, ScriptHash: scriptOut, }, }, - Scripts: []*transaction.Witness{ + Scripts: []transaction.Witness{ { InvocationScript: []byte{}, VerificationScript: []byte{byte(opcode.PUSHT)}, @@ -105,10 +105,10 @@ func governingTokenTX() *transaction.Transaction { tx := &transaction.Transaction{ Type: transaction.RegisterType, Data: registerTX, - Attributes: []*transaction.Attribute{}, - Inputs: []*transaction.Input{}, - Outputs: []*transaction.Output{}, - Scripts: []*transaction.Witness{}, + Attributes: []transaction.Attribute{}, + Inputs: []transaction.Input{}, + Outputs: []transaction.Output{}, + Scripts: []transaction.Witness{}, } return tx @@ -127,10 +127,10 @@ func utilityTokenTX() *transaction.Transaction { tx := &transaction.Transaction{ Type: transaction.RegisterType, Data: registerTX, - Attributes: []*transaction.Attribute{}, - Inputs: []*transaction.Input{}, - Outputs: []*transaction.Output{}, - Scripts: []*transaction.Witness{}, + Attributes: []transaction.Attribute{}, + Inputs: []transaction.Input{}, + Outputs: []transaction.Output{}, + Scripts: []transaction.Witness{}, } return tx diff --git a/pkg/network/payload/headers_test.go b/pkg/network/payload/headers_test.go index 78d7644c5..44c2b001f 100644 --- a/pkg/network/payload/headers_test.go +++ b/pkg/network/payload/headers_test.go @@ -16,7 +16,7 @@ func TestHeadersEncodeDecode(t *testing.T) { BlockBase: core.BlockBase{ Version: 0, Index: 1, - Script: &transaction.Witness{ + Script: transaction.Witness{ InvocationScript: []byte{0x0}, VerificationScript: []byte{0x1}, }, @@ -25,7 +25,7 @@ func TestHeadersEncodeDecode(t *testing.T) { BlockBase: core.BlockBase{ Version: 0, Index: 2, - Script: &transaction.Witness{ + Script: transaction.Witness{ InvocationScript: []byte{0x0}, VerificationScript: []byte{0x1}, }, @@ -34,7 +34,7 @@ func TestHeadersEncodeDecode(t *testing.T) { BlockBase: core.BlockBase{ Version: 0, Index: 3, - Script: &transaction.Witness{ + Script: transaction.Witness{ InvocationScript: []byte{0x0}, VerificationScript: []byte{0x1}, }, diff --git a/pkg/rpc/txBuilder.go b/pkg/rpc/txBuilder.go index 71036e341..4ce978b58 100644 --- a/pkg/rpc/txBuilder.go +++ b/pkg/rpc/txBuilder.go @@ -39,7 +39,7 @@ func CreateRawContractTransaction(params ContractTxParams) (*transaction.Transac return nil, errs.Wrapf(err, "Failed to take script hash from address: %v", address) } tx.Attributes = append(tx.Attributes, - &transaction.Attribute{ + transaction.Attribute{ Usage: transaction.Script, Data: fromAddressHash.BytesBE(), }) @@ -87,7 +87,7 @@ func SignTx(tx *transaction.Transaction, wif *keys.WIF) error { return errs.Wrap(err, "failed to create invocation script") } witness.VerificationScript = wif.PrivateKey.PublicKey().GetVerificationScript() - tx.Scripts = append(tx.Scripts, &witness) + tx.Scripts = append(tx.Scripts, witness) tx.Hash() return nil From 5b6c5af704030014471e8387c5fc27796b6a0356 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 9 Dec 2019 18:25:15 +0300 Subject: [PATCH 3/5] *: implement EncodeBinary with pointer receivers where appropriate Everywhere except ParamType (which is just a byte), reduce copying things around for no real reason. --- pkg/consensus/payload.go | 2 +- pkg/core/account_state.go | 2 +- pkg/core/block.go | 3 ++- pkg/core/notification_event.go | 2 +- pkg/util/uint256.go | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/consensus/payload.go b/pkg/consensus/payload.go index f8f635314..52ba5c4b8 100644 --- a/pkg/consensus/payload.go +++ b/pkg/consensus/payload.go @@ -160,7 +160,7 @@ func (p *Payload) SetHeight(h uint32) { } // EncodeBinaryUnsigned writes payload to w excluding signature. -func (p Payload) EncodeBinaryUnsigned(w *io.BinWriter) { +func (p *Payload) EncodeBinaryUnsigned(w *io.BinWriter) { w.WriteLE(p.version) w.WriteBytes(p.prevHash[:]) w.WriteLE(p.height) diff --git a/pkg/core/account_state.go b/pkg/core/account_state.go index daf07b805..cd79fe189 100644 --- a/pkg/core/account_state.go +++ b/pkg/core/account_state.go @@ -140,7 +140,7 @@ func (u *UnspentBalance) DecodeBinary(r *io.BinReader) { } // EncodeBinary implements io.Serializable interface. -func (u UnspentBalance) EncodeBinary(w *io.BinWriter) { +func (u *UnspentBalance) EncodeBinary(w *io.BinWriter) { u.Tx.EncodeBinary(w) w.WriteLE(u.Index) w.WriteLE(u.Value) diff --git a/pkg/core/block.go b/pkg/core/block.go index e1e898ed7..2a6af8d27 100644 --- a/pkg/core/block.go +++ b/pkg/core/block.go @@ -115,7 +115,8 @@ func (b *Block) Trim() ([]byte, error) { buf.WriteVarUint(uint64(len(b.Transactions))) for _, tx := range b.Transactions { - tx.Hash().EncodeBinary(buf.BinWriter) + h := tx.Hash() + h.EncodeBinary(buf.BinWriter) } if buf.Err != nil { return nil, buf.Err diff --git a/pkg/core/notification_event.go b/pkg/core/notification_event.go index fc1c6e727..ee0086cc3 100644 --- a/pkg/core/notification_event.go +++ b/pkg/core/notification_event.go @@ -56,7 +56,7 @@ func getAppExecResultFromStore(s storage.Store, hash util.Uint256) (*AppExecResu } // EncodeBinary implements the Serializable interface. -func (ne NotificationEvent) EncodeBinary(w *io.BinWriter) { +func (ne *NotificationEvent) EncodeBinary(w *io.BinWriter) { w.WriteBytes(ne.ScriptHash[:]) vm.EncodeBinaryStackItem(ne.Item, w) } diff --git a/pkg/util/uint256.go b/pkg/util/uint256.go index 81521e1b8..b8b5588bf 100644 --- a/pkg/util/uint256.go +++ b/pkg/util/uint256.go @@ -117,7 +117,7 @@ func (u Uint256) MarshalJSON() ([]byte, error) { func (u Uint256) CompareTo(other Uint256) int { return bytes.Compare(u[:], other[:]) } // EncodeBinary implements io.Serializable interface. -func (u Uint256) EncodeBinary(w *io.BinWriter) { +func (u *Uint256) EncodeBinary(w *io.BinWriter) { w.WriteBytes(u[:]) } From f1856bfa8b1baf408967970f61ad03f820c14d2a Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 9 Dec 2019 18:33:04 +0300 Subject: [PATCH 4/5] core/tx: remove publickey indirection from assets and txes It makes very little sense having pointers here, these structures MUST have some kind of key and this key is not gonna be wandering somewhere on its own. Fixes a part of #519. --- pkg/core/asset_state.go | 3 +-- pkg/core/asset_state_test.go | 3 --- pkg/core/blockchain.go | 2 +- pkg/core/interop_neo.go | 2 +- pkg/core/interop_neo_test.go | 2 +- pkg/core/transaction/enrollment.go | 3 +-- pkg/core/transaction/register.go | 3 +-- pkg/core/transaction/register_test.go | 3 +-- pkg/core/util.go | 2 -- 9 files changed, 7 insertions(+), 16 deletions(-) diff --git a/pkg/core/asset_state.go b/pkg/core/asset_state.go index fc0fe09e7..6af69172c 100644 --- a/pkg/core/asset_state.go +++ b/pkg/core/asset_state.go @@ -43,7 +43,7 @@ type AssetState struct { Precision uint8 FeeMode uint8 FeeAddress util.Uint160 - Owner *keys.PublicKey + Owner keys.PublicKey Admin util.Uint160 Issuer util.Uint160 Expiration uint32 @@ -63,7 +63,6 @@ func (a *AssetState) DecodeBinary(br *io.BinReader) { br.ReadLE(&a.FeeMode) br.ReadBytes(a.FeeAddress[:]) - a.Owner = &keys.PublicKey{} a.Owner.DecodeBinary(br) br.ReadBytes(a.Admin[:]) br.ReadBytes(a.Issuer[:]) diff --git a/pkg/core/asset_state_test.go b/pkg/core/asset_state_test.go index 8fbfb3791..ac9d1ef7b 100644 --- a/pkg/core/asset_state_test.go +++ b/pkg/core/asset_state_test.go @@ -5,7 +5,6 @@ import ( "github.com/CityOfZion/neo-go/pkg/core/storage" "github.com/CityOfZion/neo-go/pkg/core/transaction" - "github.com/CityOfZion/neo-go/pkg/crypto/keys" "github.com/CityOfZion/neo-go/pkg/io" "github.com/CityOfZion/neo-go/pkg/util" "github.com/stretchr/testify/assert" @@ -20,7 +19,6 @@ func TestEncodeDecodeAssetState(t *testing.T) { Available: util.Fixed8(100), Precision: 0, FeeMode: feeMode, - Owner: &keys.PublicKey{}, Admin: randomUint160(), Issuer: randomUint160(), Expiration: 10, @@ -47,7 +45,6 @@ func TestPutGetAssetState(t *testing.T) { Available: util.Fixed8(100), Precision: 8, FeeMode: feeMode, - Owner: &keys.PublicKey{}, Admin: randomUint160(), Issuer: randomUint160(), Expiration: 10, diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 6544ebc4b..0a75d9d2a 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -1340,7 +1340,7 @@ func processStateTX(chainState *BlockChainState, tx *transaction.StateTX) error } func processEnrollmentTX(chainState *BlockChainState, tx *transaction.EnrollmentTX) error { - validatorState, err := chainState.validators.getAndUpdate(chainState.store, tx.PublicKey) + validatorState, err := chainState.validators.getAndUpdate(chainState.store, &tx.PublicKey) if err != nil { return err } diff --git a/pkg/core/interop_neo.go b/pkg/core/interop_neo.go index da688b9cc..17c100e1c 100644 --- a/pkg/core/interop_neo.go +++ b/pkg/core/interop_neo.go @@ -615,7 +615,7 @@ func (ic *interopContext) assetCreate(v *vm.VM) error { Name: name, Amount: amount, Precision: precision, - Owner: owner, + Owner: *owner, Admin: admin, Issuer: issuer, Expiration: ic.bc.BlockHeight() + DefaultAssetLifetime, diff --git a/pkg/core/interop_neo_test.go b/pkg/core/interop_neo_test.go index c1c378cd9..1fee8ae98 100644 --- a/pkg/core/interop_neo_test.go +++ b/pkg/core/interop_neo_test.go @@ -348,7 +348,7 @@ func createVMAndAssetState(t *testing.T) (*vm.VM, *AssetState, *interopContext) Precision: 1, FeeMode: 1, FeeAddress: randomUint160(), - Owner: &keys.PublicKey{X: big.NewInt(1), Y: big.NewInt(1)}, + Owner: keys.PublicKey{X: big.NewInt(1), Y: big.NewInt(1)}, Admin: randomUint160(), Issuer: randomUint160(), Expiration: 10, diff --git a/pkg/core/transaction/enrollment.go b/pkg/core/transaction/enrollment.go index f61269ba9..0d7c8a0e4 100644 --- a/pkg/core/transaction/enrollment.go +++ b/pkg/core/transaction/enrollment.go @@ -12,12 +12,11 @@ import ( // The way to cancel the registration is: Spend the deposit on the address of the PublicKey. type EnrollmentTX struct { // PublicKey of the validator. - PublicKey *keys.PublicKey + PublicKey keys.PublicKey } // DecodeBinary implements Serializable interface. func (tx *EnrollmentTX) DecodeBinary(r *io.BinReader) { - tx.PublicKey = &keys.PublicKey{} tx.PublicKey.DecodeBinary(r) } diff --git a/pkg/core/transaction/register.go b/pkg/core/transaction/register.go index bba131263..2c7758e84 100644 --- a/pkg/core/transaction/register.go +++ b/pkg/core/transaction/register.go @@ -23,7 +23,7 @@ type RegisterTX struct { Precision uint8 // Public key of the owner. - Owner *keys.PublicKey + Owner keys.PublicKey Admin util.Uint160 } @@ -37,7 +37,6 @@ func (tx *RegisterTX) DecodeBinary(br *io.BinReader) { br.ReadLE(&tx.Amount) br.ReadLE(&tx.Precision) - tx.Owner = &keys.PublicKey{} tx.Owner.DecodeBinary(br) br.ReadBytes(tx.Admin[:]) diff --git a/pkg/core/transaction/register_test.go b/pkg/core/transaction/register_test.go index dd9aa0b02..7d635802d 100644 --- a/pkg/core/transaction/register_test.go +++ b/pkg/core/transaction/register_test.go @@ -21,7 +21,6 @@ func TestRegisterTX(t *testing.T) { Name: "this is some token I created", Amount: util.Fixed8FromInt64(1000000), Precision: 8, - Owner: &keys.PublicKey{}, Admin: someuint160, }, } @@ -58,7 +57,7 @@ func TestDecodeRegisterTXFromRawString(t *testing.T) { assert.Equal(t, "[{\"lang\":\"zh-CN\",\"name\":\"小蚁股\"},{\"lang\":\"en\",\"name\":\"AntShare\"}]", txData.Name) assert.Equal(t, util.Fixed8FromInt64(100000000), txData.Amount) assert.Equal(t, uint8(0), txData.Precision) - assert.Equal(t, &keys.PublicKey{}, txData.Owner) + assert.Equal(t, keys.PublicKey{}, txData.Owner) assert.Equal(t, "Abf2qMs1pzQb8kYk9RuxtUb9jtRKJVuBJt", crypto.AddressFromUint160(txData.Admin)) assert.Equal(t, "c56f33fc6ecfcd0c225c4ab356fee59390af8560be0e930faebe74a6daff7c9b", tx.Hash().StringLE()) diff --git a/pkg/core/util.go b/pkg/core/util.go index 8da60beab..c553f5ef0 100644 --- a/pkg/core/util.go +++ b/pkg/core/util.go @@ -98,7 +98,6 @@ func governingTokenTX() *transaction.Transaction { Name: "[{\"lang\":\"zh-CN\",\"name\":\"小蚁股\"},{\"lang\":\"en\",\"name\":\"AntShare\"}]", Amount: util.Fixed8FromInt64(100000000), Precision: 0, - Owner: &keys.PublicKey{}, Admin: admin, } @@ -121,7 +120,6 @@ func utilityTokenTX() *transaction.Transaction { Name: "[{\"lang\":\"zh-CN\",\"name\":\"小蚁币\"},{\"lang\":\"en\",\"name\":\"AntCoin\"}]", Amount: calculateUtilityAmount(), Precision: 8, - Owner: &keys.PublicKey{}, Admin: admin, } tx := &transaction.Transaction{ From 35e368c24168e75c9a4ab2122b1a96e11e09294c Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 9 Dec 2019 18:39:30 +0300 Subject: [PATCH 5/5] io: add a note for WriteArray, fix #519 It can't be really solved in many cases (it's used in P2P protocol and we have to follow the usual conventions there) and in most of the cases we don't care about the difference between nil slice and zero-length slice. --- pkg/io/binaryWriter.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/io/binaryWriter.go b/pkg/io/binaryWriter.go index d9ea336e2..457f7f846 100644 --- a/pkg/io/binaryWriter.go +++ b/pkg/io/binaryWriter.go @@ -35,7 +35,9 @@ func (w *BinWriter) WriteBE(v interface{}) { w.Err = binary.Write(w.w, binary.BigEndian, v) } -// WriteArray writes a slice or an array arr into w. +// WriteArray writes a slice or an array arr into w. Note that nil slices and +// empty slices are gonna be treated the same resulting in equal zero-length +// array encoded. func (w *BinWriter) WriteArray(arr interface{}) { switch val := reflect.ValueOf(arr); val.Kind() { case reflect.Slice, reflect.Array: