From df2a56908b0a4f99c76e1810030b1d675ca10f26 Mon Sep 17 00:00:00 2001 From: Ekaterina Pavlova Date: Fri, 26 Apr 2024 21:35:57 +0530 Subject: [PATCH 1/5] core: move transaction Attribute value to a designated interface Signed-off-by: Ekaterina Pavlova --- pkg/core/transaction/attribute.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/pkg/core/transaction/attribute.go b/pkg/core/transaction/attribute.go index 0044f4b5f..187c8b185 100644 --- a/pkg/core/transaction/attribute.go +++ b/pkg/core/transaction/attribute.go @@ -8,17 +8,20 @@ import ( "github.com/nspcc-dev/neo-go/pkg/io" ) +// AttrValue represents a Transaction Attribute value. +type AttrValue interface { + io.Serializable + // toJSONMap is used for embedded json struct marshalling. + // Anonymous interface fields are not considered anonymous by + // json lib and marshaling Value together with type makes code + // harder to follow. + toJSONMap(map[string]any) +} + // Attribute represents a Transaction attribute. type Attribute struct { Type AttrType - Value interface { - io.Serializable - // toJSONMap is used for embedded json struct marshalling. - // Anonymous interface fields are not considered anonymous by - // json lib and marshaling Value together with type makes code - // harder to follow. - toJSONMap(map[string]any) - } + Value AttrValue } // attrJSON is used for JSON I/O of Attribute. From 7c8d2c3ec58d92a25d7ad5d776b700bd56b524e0 Mon Sep 17 00:00:00 2001 From: Ekaterina Pavlova Date: Sat, 27 Apr 2024 14:48:48 +0530 Subject: [PATCH 2/5] crypto: add nil check for PublicKeys Copy() Signed-off-by: Ekaterina Pavlova --- pkg/crypto/keys/publickey.go | 6 +++++- pkg/crypto/keys/publickey_test.go | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/crypto/keys/publickey.go b/pkg/crypto/keys/publickey.go index 5d1e5ce41..774623456 100644 --- a/pkg/crypto/keys/publickey.go +++ b/pkg/crypto/keys/publickey.go @@ -78,8 +78,12 @@ func (keys PublicKeys) Contains(pKey *PublicKey) bool { return false } -// Copy returns a copy of keys. +// Copy returns a shallow copy of the PublicKeys slice. It creates a new slice with the same elements, +// but does not perform a deep copy of the elements themselves. func (keys PublicKeys) Copy() PublicKeys { + if keys == nil { + return nil + } res := make(PublicKeys, len(keys)) copy(res, keys) return res diff --git a/pkg/crypto/keys/publickey_test.go b/pkg/crypto/keys/publickey_test.go index d22c0d090..1f05c7b30 100644 --- a/pkg/crypto/keys/publickey_test.go +++ b/pkg/crypto/keys/publickey_test.go @@ -40,6 +40,8 @@ func TestEncodeDecodePublicKey(t *testing.T) { } func TestPublicKeys_Copy(t *testing.T) { + require.Nil(t, (PublicKeys)(nil).Copy()) + pubs := make(PublicKeys, 5) for i := range pubs { priv, err := NewPrivateKey() From 1292a00ef95ba4a10c79a80e0a0f03f911e22ad2 Mon Sep 17 00:00:00 2001 From: Ekaterina Pavlova Date: Sun, 28 Apr 2024 00:06:17 +0530 Subject: [PATCH 3/5] crypto: adjust TestPublicKeys_Copy Since the AllowedGroups []*keys.PublicKey slice is used in the initialization, the test should use the same structures. Signed-off-by: Ekaterina Pavlova --- pkg/crypto/keys/publickey_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/crypto/keys/publickey_test.go b/pkg/crypto/keys/publickey_test.go index 1f05c7b30..1f9e5bba9 100644 --- a/pkg/crypto/keys/publickey_test.go +++ b/pkg/crypto/keys/publickey_test.go @@ -42,14 +42,18 @@ func TestEncodeDecodePublicKey(t *testing.T) { func TestPublicKeys_Copy(t *testing.T) { require.Nil(t, (PublicKeys)(nil).Copy()) - pubs := make(PublicKeys, 5) - for i := range pubs { + pubz := make([]*PublicKey, 5) + for i := range pubz { priv, err := NewPrivateKey() require.NoError(t, err) - pubs[i] = priv.PublicKey() + pubz[i] = priv.PublicKey() } + pubs := PublicKeys(pubz) cp := pubs.Copy() + var pubx = ([]*PublicKey)(cp) + require.Equal(t, pubz, pubx) + priv, err := NewPrivateKey() require.NoError(t, err) cp[0] = priv.PublicKey() From 956fd08adb6dab02af0aeb39195c7985b3cfb69e Mon Sep 17 00:00:00 2001 From: Ekaterina Pavlova Date: Sat, 27 Apr 2024 22:46:05 +0530 Subject: [PATCH 4/5] *: add Copy() to transaction.Transaction and payload.P2PNotaryRequest Add a method that makes a deep copy of all fields and resets size/hash caches. Close #3288 Signed-off-by: Ekaterina Pavlova --- pkg/core/transaction/attribute.go | 16 +++++ pkg/core/transaction/attribute_test.go | 37 ++++++++++ pkg/core/transaction/conflicts.go | 7 ++ pkg/core/transaction/not_valid_before.go | 7 ++ pkg/core/transaction/notary_assisted.go | 7 ++ pkg/core/transaction/oracle.go | 10 +++ pkg/core/transaction/reserved.go | 7 ++ pkg/core/transaction/signer.go | 21 ++++++ pkg/core/transaction/signer_test.go | 37 ++++++++++ pkg/core/transaction/transaction.go | 34 ++++++++++ pkg/core/transaction/transaction_test.go | 66 ++++++++++++++++++ pkg/core/transaction/witness.go | 10 +++ pkg/core/transaction/witness_condition.go | 61 +++++++++++++++++ .../transaction/witness_condition_test.go | 68 +++++++++++++++++++ pkg/core/transaction/witness_rule.go | 8 +++ pkg/core/transaction/witness_rule_test.go | 12 ++++ pkg/core/transaction/witness_test.go | 13 ++++ pkg/network/payload/notary_request.go | 12 ++++ pkg/network/payload/notary_request_test.go | 66 ++++++++++++++++++ 19 files changed, 499 insertions(+) diff --git a/pkg/core/transaction/attribute.go b/pkg/core/transaction/attribute.go index 187c8b185..68f0657cb 100644 --- a/pkg/core/transaction/attribute.go +++ b/pkg/core/transaction/attribute.go @@ -16,6 +16,8 @@ type AttrValue interface { // json lib and marshaling Value together with type makes code // harder to follow. toJSONMap(map[string]any) + // Copy returns a deep copy of the attribute value. + Copy() AttrValue } // Attribute represents a Transaction attribute. @@ -110,3 +112,17 @@ func (attr *Attribute) UnmarshalJSON(data []byte) error { } return json.Unmarshal(data, attr.Value) } + +// Copy creates a deep copy of the Attribute. +func (attr *Attribute) Copy() *Attribute { + if attr == nil { + return nil + } + cp := &Attribute{ + Type: attr.Type, + } + if attr.Value != nil { + cp.Value = attr.Value.Copy() + } + return cp +} diff --git a/pkg/core/transaction/attribute_test.go b/pkg/core/transaction/attribute_test.go index f35a79732..28ad13cb0 100644 --- a/pkg/core/transaction/attribute_test.go +++ b/pkg/core/transaction/attribute_test.go @@ -9,6 +9,7 @@ import ( "github.com/nspcc-dev/neo-go/internal/testserdes" "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/util" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -177,3 +178,39 @@ func TestAttribute_MarshalJSON(t *testing.T) { testserdes.MarshalUnmarshalJSON(t, attr, new(Attribute)) }) } + +func TestAttribute_Copy(t *testing.T) { + originals := []*Attribute{ + {Type: HighPriority}, + {Type: OracleResponseT, Value: &OracleResponse{ID: 123, Code: Success, Result: []byte{1, 2, 3}}}, + {Type: NotValidBeforeT, Value: &NotValidBefore{Height: 123}}, + {Type: ConflictsT, Value: &Conflicts{Hash: random.Uint256()}}, + {Type: NotaryAssistedT, Value: &NotaryAssisted{NKeys: 3}}, + {Type: ReservedLowerBound, Value: &Reserved{Value: []byte{1, 2, 3, 4, 5}}}, + {Type: ReservedUpperBound, Value: &Reserved{Value: []byte{1, 2, 3, 4, 5}}}, + {Type: ReservedLowerBound + 5, Value: &Reserved{Value: []byte{1, 2, 3, 4, 5}}}, + } + require.Nil(t, (*Attribute)(nil).Copy()) + + for _, original := range originals { + cp := original.Copy() + require.NotNil(t, cp) + assert.Equal(t, original.Type, cp.Type) + assert.Equal(t, original.Value, cp.Value) + if original.Value != nil { + originalValueCopy := original.Value.Copy() + switch originalVal := originalValueCopy.(type) { + case *OracleResponse: + originalVal.Result = append(originalVal.Result, 0xFF) + assert.NotEqual(t, len(original.Value.(*OracleResponse).Result), len(originalVal.Result)) + case *Conflicts: + originalVal.Hash = random.Uint256() + assert.NotEqual(t, original.Value.(*Conflicts).Hash, originalVal.Hash) + case *NotaryAssisted: + originalVal.NKeys++ + assert.NotEqual(t, original.Value.(*NotaryAssisted).NKeys, originalVal.NKeys) + } + assert.Equal(t, cp.Value, original.Value) + } + } +} diff --git a/pkg/core/transaction/conflicts.go b/pkg/core/transaction/conflicts.go index 50005fb21..a085c4a1c 100644 --- a/pkg/core/transaction/conflicts.go +++ b/pkg/core/transaction/conflicts.go @@ -23,3 +23,10 @@ func (c *Conflicts) EncodeBinary(w *io.BinWriter) { func (c *Conflicts) toJSONMap(m map[string]any) { m["hash"] = c.Hash } + +// Copy implements the AttrValue interface. +func (c *Conflicts) Copy() AttrValue { + return &Conflicts{ + Hash: c.Hash, + } +} diff --git a/pkg/core/transaction/not_valid_before.go b/pkg/core/transaction/not_valid_before.go index 6fe9b7b56..9d902aee0 100644 --- a/pkg/core/transaction/not_valid_before.go +++ b/pkg/core/transaction/not_valid_before.go @@ -22,3 +22,10 @@ func (n *NotValidBefore) EncodeBinary(w *io.BinWriter) { func (n *NotValidBefore) toJSONMap(m map[string]any) { m["height"] = n.Height } + +// Copy implements the AttrValue interface. +func (n *NotValidBefore) Copy() AttrValue { + return &NotValidBefore{ + Height: n.Height, + } +} diff --git a/pkg/core/transaction/notary_assisted.go b/pkg/core/transaction/notary_assisted.go index 747711165..278aaea93 100644 --- a/pkg/core/transaction/notary_assisted.go +++ b/pkg/core/transaction/notary_assisted.go @@ -22,3 +22,10 @@ func (n *NotaryAssisted) EncodeBinary(w *io.BinWriter) { func (n *NotaryAssisted) toJSONMap(m map[string]any) { m["nkeys"] = n.NKeys } + +// Copy implements the AttrValue interface. +func (n *NotaryAssisted) Copy() AttrValue { + return &NotaryAssisted{ + NKeys: n.NKeys, + } +} diff --git a/pkg/core/transaction/oracle.go b/pkg/core/transaction/oracle.go index 68459ef56..6c51c7ed9 100644 --- a/pkg/core/transaction/oracle.go +++ b/pkg/core/transaction/oracle.go @@ -1,6 +1,7 @@ package transaction import ( + "bytes" "encoding/json" "errors" "math" @@ -116,3 +117,12 @@ func (r *OracleResponse) toJSONMap(m map[string]any) { m["code"] = r.Code m["result"] = r.Result } + +// Copy implements the AttrValue interface. +func (r *OracleResponse) Copy() AttrValue { + return &OracleResponse{ + ID: r.ID, + Code: r.Code, + Result: bytes.Clone(r.Result), + } +} diff --git a/pkg/core/transaction/reserved.go b/pkg/core/transaction/reserved.go index 677458aa3..61ae31152 100644 --- a/pkg/core/transaction/reserved.go +++ b/pkg/core/transaction/reserved.go @@ -22,3 +22,10 @@ func (e *Reserved) EncodeBinary(w *io.BinWriter) { func (e *Reserved) toJSONMap(m map[string]any) { m["value"] = e.Value } + +// Copy implements the AttrValue interface. +func (e *Reserved) Copy() AttrValue { + return &Reserved{ + Value: e.Value, + } +} diff --git a/pkg/core/transaction/signer.go b/pkg/core/transaction/signer.go index 663513925..a4290b66b 100644 --- a/pkg/core/transaction/signer.go +++ b/pkg/core/transaction/signer.go @@ -86,3 +86,24 @@ func SignersToStackItem(signers []Signer) stackitem.Item { } return stackitem.NewArray(res) } + +// Copy creates a deep copy of the Signer. +func (c *Signer) Copy() *Signer { + if c == nil { + return nil + } + cp := *c + if c.AllowedContracts != nil { + cp.AllowedContracts = make([]util.Uint160, len(c.AllowedContracts)) + copy(cp.AllowedContracts, c.AllowedContracts) + } + cp.AllowedGroups = keys.PublicKeys(c.AllowedGroups).Copy() + if c.Rules != nil { + cp.Rules = make([]WitnessRule, len(c.Rules)) + for i, rule := range c.Rules { + cp.Rules[i] = *rule.Copy() + } + } + + return &cp +} diff --git a/pkg/core/transaction/signer_test.go b/pkg/core/transaction/signer_test.go index 58242a8e9..ced81320b 100644 --- a/pkg/core/transaction/signer_test.go +++ b/pkg/core/transaction/signer_test.go @@ -32,3 +32,40 @@ func TestCosignerMarshallUnmarshallJSON(t *testing.T) { actual := &Signer{} testserdes.MarshalUnmarshalJSON(t, expected, actual) } + +func TestSignerCopy(t *testing.T) { + pk, err := keys.NewPrivateKey() + require.NoError(t, err) + require.Nil(t, (*Signer)(nil).Copy()) + + original := &Signer{ + Account: util.Uint160{1, 2, 3, 4, 5}, + Scopes: CustomContracts | CustomGroups | Rules, + AllowedContracts: []util.Uint160{{1, 2, 3, 4}, {6, 7, 8, 9}}, + AllowedGroups: keys.PublicKeys{pk.PublicKey()}, + Rules: []WitnessRule{{Action: WitnessAllow, Condition: ConditionCalledByEntry{}}}, + } + + cp := original.Copy() + require.NotNil(t, cp, "Copied Signer should not be nil") + + require.Equal(t, original.Account, cp.Account) + require.Equal(t, original.Scopes, cp.Scopes) + + require.NotSame(t, original.AllowedContracts, cp.AllowedContracts) + require.Equal(t, original.AllowedContracts, cp.AllowedContracts) + + require.NotSame(t, original.AllowedGroups, cp.AllowedGroups) + require.Equal(t, original.AllowedGroups, cp.AllowedGroups) + + require.NotSame(t, original.Rules, cp.Rules) + require.Equal(t, original.Rules, cp.Rules) + + original.AllowedContracts[0][0] = 255 + original.AllowedGroups[0] = nil + original.Rules[0].Action = WitnessDeny + + require.NotEqual(t, original.AllowedContracts[0][0], cp.AllowedContracts[0][0]) + require.NotEqual(t, original.AllowedGroups[0], cp.AllowedGroups[0]) + require.NotEqual(t, original.Rules[0].Action, cp.Rules[0].Action) +} diff --git a/pkg/core/transaction/transaction.go b/pkg/core/transaction/transaction.go index eba0c237d..a41c811a4 100644 --- a/pkg/core/transaction/transaction.go +++ b/pkg/core/transaction/transaction.go @@ -1,6 +1,7 @@ package transaction import ( + "bytes" "crypto/sha256" "encoding/json" "errors" @@ -468,3 +469,36 @@ func (t *Transaction) ToStackItem() stackitem.Item { stackitem.NewByteArray(t.Script), }) } + +// Copy creates a deep copy of the Transaction, including all slice fields. Cached values like +// 'hashed' and 'size' are reset to ensure the copy can be modified independently of the original. +func (t *Transaction) Copy() *Transaction { + if t == nil { + return nil + } + cp := *t + if t.Attributes != nil { + cp.Attributes = make([]Attribute, len(t.Attributes)) + for i, attr := range t.Attributes { + cp.Attributes[i] = *attr.Copy() + } + } + if t.Signers != nil { + cp.Signers = make([]Signer, len(t.Signers)) + for i, signer := range t.Signers { + cp.Signers[i] = *signer.Copy() + } + } + if t.Scripts != nil { + cp.Scripts = make([]Witness, len(t.Scripts)) + for i, script := range t.Scripts { + cp.Scripts[i] = script.Copy() + } + } + cp.Script = bytes.Clone(t.Script) + + cp.hashed = false + cp.size = 0 + cp.hash = util.Uint256{} + return &cp +} diff --git a/pkg/core/transaction/transaction_test.go b/pkg/core/transaction/transaction_test.go index 9f8340bc4..c99f2790f 100644 --- a/pkg/core/transaction/transaction_test.go +++ b/pkg/core/transaction/transaction_test.go @@ -9,6 +9,7 @@ import ( "github.com/nspcc-dev/neo-go/internal/random" "github.com/nspcc-dev/neo-go/internal/testserdes" + "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/encoding/fixedn" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm/opcode" @@ -321,3 +322,68 @@ func BenchmarkTxHash(b *testing.B) { _ = tx.Hash() } } + +func TestTransaction_DeepCopy(t *testing.T) { + origTx := New([]byte{0x01, 0x02, 0x03}, 1000) + origTx.NetworkFee = 2000 + origTx.SystemFee = 500 + origTx.Nonce = 12345678 + origTx.ValidUntilBlock = 100 + origTx.Version = 1 + require.Nil(t, (*Transaction)(nil).Copy()) + + priv, err := keys.NewPrivateKey() + require.NoError(t, err) + origTx.Signers = []Signer{ + {Account: random.Uint160(), Scopes: Global, AllowedContracts: []util.Uint160{random.Uint160()}, AllowedGroups: keys.PublicKeys{priv.PublicKey()}, Rules: []WitnessRule{{Action: 0x01, Condition: ConditionCalledByEntry{}}}}, + {Account: random.Uint160(), Scopes: CalledByEntry}, + } + origTx.Attributes = []Attribute{ + {Type: HighPriority, Value: &OracleResponse{ + ID: 0, + Code: Success, + Result: []byte{4, 8, 15, 16, 23, 42}, + }}, + } + origTx.Scripts = []Witness{ + { + InvocationScript: []byte{0x04, 0x05}, + VerificationScript: []byte{0x06, 0x07}, + }, + } + origTxHash := origTx.Hash() + + copyTx := origTx.Copy() + + require.Equal(t, origTx.Hash(), copyTx.Hash()) + require.Equal(t, origTx, copyTx) + require.Equal(t, origTx.Size(), copyTx.Size()) + + copyTx.NetworkFee = 3000 + copyTx.Signers[0].Scopes = None + copyTx.Attributes[0].Type = NotaryAssistedT + copyTx.Scripts[0].InvocationScript[0] = 0x08 + copyTx.hashed = false + modifiedCopyTxHash := copyTx.Hash() + + require.NotEqual(t, origTx.NetworkFee, copyTx.NetworkFee) + require.NotEqual(t, origTx.Signers[0].Scopes, copyTx.Signers[0].Scopes) + require.NotEqual(t, origTx.Attributes, copyTx.Attributes) + require.NotEqual(t, origTxHash, modifiedCopyTxHash) + + require.NotEqual(t, &origTx.Scripts[0].InvocationScript[0], ©Tx.Scripts[0].InvocationScript[0]) + require.NotEqual(t, &origTx.Scripts, ©Tx.Scripts) + require.Equal(t, origTx.Scripts[0].VerificationScript, copyTx.Scripts[0].VerificationScript) + require.Equal(t, origTx.Signers[0].AllowedContracts, copyTx.Signers[0].AllowedContracts) + require.Equal(t, origTx.Signers[0].AllowedGroups, copyTx.Signers[0].AllowedGroups) + origGroup := origTx.Signers[0].AllowedGroups[0] + copyGroup := copyTx.Signers[0].AllowedGroups[0] + require.True(t, origGroup.Equal(copyGroup)) + + copyTx.Signers[0].AllowedGroups[0] = nil + require.NotEqual(t, origTx.Signers[0].AllowedGroups[0], copyTx.Signers[0].AllowedGroups[0]) + + require.Equal(t, origTx.Signers[0].Rules[0], copyTx.Signers[0].Rules[0]) + copyTx.Signers[0].Rules[0].Action = 0x02 + require.NotEqual(t, origTx.Signers[0].Rules[0].Action, copyTx.Signers[0].Rules[0].Action) +} diff --git a/pkg/core/transaction/witness.go b/pkg/core/transaction/witness.go index dfa441b25..ae4f2446c 100644 --- a/pkg/core/transaction/witness.go +++ b/pkg/core/transaction/witness.go @@ -1,6 +1,8 @@ package transaction import ( + "bytes" + "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/util" @@ -38,3 +40,11 @@ func (w *Witness) EncodeBinary(bw *io.BinWriter) { func (w Witness) ScriptHash() util.Uint160 { return hash.Hash160(w.VerificationScript) } + +// Copy creates a deep copy of the Witness. +func (w Witness) Copy() Witness { + return Witness{ + InvocationScript: bytes.Clone(w.InvocationScript), + VerificationScript: bytes.Clone(w.VerificationScript), + } +} diff --git a/pkg/core/transaction/witness_condition.go b/pkg/core/transaction/witness_condition.go index b34a8d389..d639cf300 100644 --- a/pkg/core/transaction/witness_condition.go +++ b/pkg/core/transaction/witness_condition.go @@ -54,6 +54,8 @@ type WitnessCondition interface { DecodeBinarySpecific(*io.BinReader, int) // ToStackItem converts WitnessCondition to stackitem.Item. ToStackItem() stackitem.Item + // Copy returns a deep copy of the condition. + Copy() WitnessCondition json.Marshaler } @@ -139,6 +141,12 @@ func (c *ConditionBoolean) ToStackItem() stackitem.Item { return condToStackItem(c.Type(), bool(*c)) } +// Copy returns a deep copy of the condition. +func (c *ConditionBoolean) Copy() WitnessCondition { + cc := *c + return &cc +} + // Type implements the WitnessCondition interface and returns condition type. func (c *ConditionNot) Type() WitnessConditionType { return WitnessNot @@ -182,6 +190,12 @@ func (c *ConditionNot) ToStackItem() stackitem.Item { return condToStackItem(c.Type(), c.Condition) } +// Copy implements the WitnessCondition interface and returns a deep copy of the condition. +func (c *ConditionNot) Copy() WitnessCondition { + cp := *c + return &cp +} + // Type implements the WitnessCondition interface and returns condition type. func (c *ConditionAnd) Type() WitnessConditionType { return WitnessAnd @@ -264,6 +278,15 @@ func (c *ConditionAnd) ToStackItem() stackitem.Item { return condToStackItem(c.Type(), []WitnessCondition(*c)) } +// Copy implements the WitnessCondition interface and returns a deep copy of the condition. +func (c *ConditionAnd) Copy() WitnessCondition { + cp := make(ConditionAnd, len(*c)) + for i, cond := range *c { + cp[i] = cond.Copy() + } + return &cp +} + // Type implements the WitnessCondition interface and returns condition type. func (c *ConditionOr) Type() WitnessConditionType { return WitnessOr @@ -310,6 +333,15 @@ func (c *ConditionOr) ToStackItem() stackitem.Item { return condToStackItem(c.Type(), []WitnessCondition(*c)) } +// Copy implements the WitnessCondition interface and returns a deep copy of the condition. +func (c *ConditionOr) Copy() WitnessCondition { + cp := make(ConditionOr, len(*c)) + for i, cond := range *c { + cp[i] = cond.Copy() + } + return &cp +} + // Type implements the WitnessCondition interface and returns condition type. func (c *ConditionScriptHash) Type() WitnessConditionType { return WitnessScriptHash @@ -348,6 +380,12 @@ func (c *ConditionScriptHash) ToStackItem() stackitem.Item { return condToStackItem(c.Type(), util.Uint160(*c)) } +// Copy implements the WitnessCondition interface and returns a deep copy of the condition. +func (c *ConditionScriptHash) Copy() WitnessCondition { + cc := *c + return &cc +} + // Type implements the WitnessCondition interface and returns condition type. func (c *ConditionGroup) Type() WitnessConditionType { return WitnessGroup @@ -386,6 +424,12 @@ func (c *ConditionGroup) ToStackItem() stackitem.Item { return condToStackItem(c.Type(), keys.PublicKey(*c)) } +// Copy implements the WitnessCondition interface and returns a deep copy of the condition. +func (c *ConditionGroup) Copy() WitnessCondition { + cp := *c + return &cp +} + // Type implements the WitnessCondition interface and returns condition type. func (c ConditionCalledByEntry) Type() WitnessConditionType { return WitnessCalledByEntry @@ -421,6 +465,11 @@ func (c ConditionCalledByEntry) ToStackItem() stackitem.Item { return condToStackItem(c.Type(), nil) } +// Copy implements the WitnessCondition interface and returns a deep copy of the condition. +func (c ConditionCalledByEntry) Copy() WitnessCondition { + return ConditionCalledByEntry{} +} + // Type implements the WitnessCondition interface and returns condition type. func (c *ConditionCalledByContract) Type() WitnessConditionType { return WitnessCalledByContract @@ -459,6 +508,12 @@ func (c *ConditionCalledByContract) ToStackItem() stackitem.Item { return condToStackItem(c.Type(), util.Uint160(*c)) } +// Copy implements the WitnessCondition interface and returns a deep copy of the condition. +func (c *ConditionCalledByContract) Copy() WitnessCondition { + cc := *c + return &cc +} + // Type implements the WitnessCondition interface and returns condition type. func (c *ConditionCalledByGroup) Type() WitnessConditionType { return WitnessCalledByGroup @@ -497,6 +552,12 @@ func (c *ConditionCalledByGroup) ToStackItem() stackitem.Item { return condToStackItem(c.Type(), keys.PublicKey(*c)) } +// Copy implements the WitnessCondition interface and returns a deep copy of the condition. +func (c *ConditionCalledByGroup) Copy() WitnessCondition { + cp := *c + return &cp +} + // DecodeBinaryCondition decodes and returns condition from the given binary stream. func DecodeBinaryCondition(r *io.BinReader) WitnessCondition { return decodeBinaryCondition(r, MaxConditionNesting) diff --git a/pkg/core/transaction/witness_condition_test.go b/pkg/core/transaction/witness_condition_test.go index 2a4f56244..49fc2ad1e 100644 --- a/pkg/core/transaction/witness_condition_test.go +++ b/pkg/core/transaction/witness_condition_test.go @@ -36,6 +36,11 @@ func (c InvalidCondition) ToStackItem() stackitem.Item { panic("invalid") } +// Copy implements the WitnessCondition interface and returns a deep copy of the condition. +func (c InvalidCondition) Copy() WitnessCondition { + return c +} + type condCase struct { condition WitnessCondition success bool @@ -347,3 +352,66 @@ func TestWitnessConditionMatch(t *testing.T) { require.Error(t, err) }) } + +func TestWitnessConditionCopy(t *testing.T) { + var someBool = true + boolCondition := (*ConditionBoolean)(&someBool) + pk, err := keys.NewPrivateKey() + require.NoError(t, err) + + conditions := []WitnessCondition{ + boolCondition, + &ConditionNot{Condition: boolCondition}, + &ConditionAnd{boolCondition, boolCondition}, + &ConditionOr{boolCondition, boolCondition}, + &ConditionScriptHash{1, 2, 3}, + (*ConditionGroup)(pk.PublicKey()), + ConditionCalledByEntry{}, + &ConditionCalledByContract{1, 2, 3}, + (*ConditionCalledByGroup)(pk.PublicKey()), + &ConditionNot{Condition: &ConditionNot{Condition: &ConditionNot{Condition: boolCondition}}}, + } + for _, cond := range conditions { + copied := cond.Copy() + require.Equal(t, cond, copied) + + switch c := copied.(type) { + case *ConditionBoolean: + require.NotSame(t, c, cond.(*ConditionBoolean)) + case *ConditionScriptHash: + c[0]++ + require.NotEqual(t, c, cond.(*ConditionScriptHash)) + case *ConditionGroup: + c = (*ConditionGroup)(pk.PublicKey()) + require.NotSame(t, c, cond.(*ConditionGroup)) + newPk, _ := keys.NewPrivateKey() + copied = (*ConditionGroup)(newPk.PublicKey()) + require.NotEqual(t, copied, cond) + case *ConditionCalledByContract: + c[0]++ + require.NotEqual(t, c, cond.(*ConditionCalledByContract)) + case *ConditionCalledByGroup: + c = (*ConditionCalledByGroup)(pk.PublicKey()) + require.NotSame(t, c, cond.(*ConditionCalledByGroup)) + newPk, _ := keys.NewPrivateKey() + copied = (*ConditionCalledByGroup)(newPk.PublicKey()) + require.NotEqual(t, copied, cond) + case *ConditionNot: + require.NotSame(t, copied, cond) + copied.(*ConditionNot).Condition = ConditionCalledByEntry{} + require.NotEqual(t, copied, cond) + case *ConditionAnd: + require.NotSame(t, copied, cond) + (*(copied.(*ConditionAnd)))[0] = ConditionCalledByEntry{} + require.NotEqual(t, copied, cond) + case *ConditionOr: + require.NotSame(t, copied, cond) + (*(copied.(*ConditionOr)))[0] = ConditionCalledByEntry{} + require.NotEqual(t, copied, cond) + case *ConditionCalledByEntry: + require.NotSame(t, copied, cond) + copied = ConditionCalledByEntry{} + require.NotEqual(t, copied, cond) + } + } +} diff --git a/pkg/core/transaction/witness_rule.go b/pkg/core/transaction/witness_rule.go index 05661f196..ed1513765 100644 --- a/pkg/core/transaction/witness_rule.go +++ b/pkg/core/transaction/witness_rule.go @@ -94,3 +94,11 @@ func (w *WitnessRule) ToStackItem() stackitem.Item { w.Condition.ToStackItem(), }) } + +// Copy creates a deep copy of the WitnessRule. +func (w *WitnessRule) Copy() *WitnessRule { + return &WitnessRule{ + Action: w.Action, + Condition: w.Condition.Copy(), + } +} diff --git a/pkg/core/transaction/witness_rule_test.go b/pkg/core/transaction/witness_rule_test.go index bde78568a..f2102fa68 100644 --- a/pkg/core/transaction/witness_rule_test.go +++ b/pkg/core/transaction/witness_rule_test.go @@ -73,3 +73,15 @@ func TestWitnessRule_ToStackItem(t *testing.T) { require.Equal(t, expected, actual, act) } } + +func TestWitnessRule_Copy(t *testing.T) { + b := true + wr := &WitnessRule{ + Action: WitnessDeny, + Condition: (*ConditionBoolean)(&b), + } + copied := wr.Copy() + require.Equal(t, wr.Action, copied.Action) + require.Equal(t, wr.Condition, copied.Condition) + require.NotSame(t, wr.Condition, copied.Condition) +} diff --git a/pkg/core/transaction/witness_test.go b/pkg/core/transaction/witness_test.go index a8b945c12..1c05beaa8 100644 --- a/pkg/core/transaction/witness_test.go +++ b/pkg/core/transaction/witness_test.go @@ -36,3 +36,16 @@ func TestWitnessSerDes(t *testing.T) { require.Error(t, testserdes.DecodeBinary(bin1, exp)) require.Error(t, testserdes.DecodeBinary(bin2, exp)) } + +func TestWitnessCopy(t *testing.T) { + original := &Witness{ + InvocationScript: []byte{1, 2, 3}, + VerificationScript: []byte{3, 2, 1}, + } + + cp := original.Copy() + require.Equal(t, *original, cp) + + original.InvocationScript[0] = 0x05 + require.NotEqual(t, *original, cp) +} diff --git a/pkg/network/payload/notary_request.go b/pkg/network/payload/notary_request.go index 956b00e71..eadd6db62 100644 --- a/pkg/network/payload/notary_request.go +++ b/pkg/network/payload/notary_request.go @@ -136,3 +136,15 @@ func (r *P2PNotaryRequest) isValid() error { } return nil } + +// Copy creates a deep copy of P2PNotaryRequest. It creates deep copy of the MainTransaction, +// FallbackTransaction and Witness, including all slice fields. Cached values like +// 'hashed' and 'size' of the transactions are reset to ensure the copy can be modified +// independently of the original. +func (r *P2PNotaryRequest) Copy() *P2PNotaryRequest { + return &P2PNotaryRequest{ + MainTransaction: r.MainTransaction.Copy(), + FallbackTransaction: r.FallbackTransaction.Copy(), + Witness: r.Witness.Copy(), + } +} diff --git a/pkg/network/payload/notary_request_test.go b/pkg/network/payload/notary_request_test.go index 41eb073f8..f6aa7c9cf 100644 --- a/pkg/network/payload/notary_request_test.go +++ b/pkg/network/payload/notary_request_test.go @@ -186,3 +186,69 @@ func TestNotaryRequestBytesFromBytes(t *testing.T) { require.NoError(t, err) require.Equal(t, p, actual) } + +func TestP2PNotaryRequest_Copy(t *testing.T) { + priv, err := keys.NewPrivateKey() + require.NoError(t, err) + orig := &P2PNotaryRequest{ + MainTransaction: &transaction.Transaction{ + NetworkFee: 2000, + SystemFee: 500, + Nonce: 12345678, + ValidUntilBlock: 100, + Version: 1, + Signers: []transaction.Signer{ + {Account: random.Uint160(), Scopes: transaction.Global, AllowedContracts: []util.Uint160{random.Uint160()}, AllowedGroups: keys.PublicKeys{priv.PublicKey()}, Rules: []transaction.WitnessRule{{Action: 0x01, Condition: transaction.ConditionCalledByEntry{}}}}, + {Account: random.Uint160(), Scopes: transaction.CalledByEntry}, + }, + + Attributes: []transaction.Attribute{ + {Type: transaction.HighPriority, Value: &transaction.OracleResponse{ + ID: 0, + Code: transaction.Success, + Result: []byte{4, 8, 15, 16, 23, 42}, + }}, + }, + Scripts: []transaction.Witness{ + { + InvocationScript: []byte{0x04, 0x05}, + VerificationScript: []byte{0x06, 0x07}, + }, + }}, + FallbackTransaction: &transaction.Transaction{ + Version: 2, + SystemFee: 200, + NetworkFee: 100, + Script: []byte{3, 2, 1}, + Signers: []transaction.Signer{{Account: util.Uint160{4, 5, 6}}}, + Attributes: []transaction.Attribute{{Type: transaction.NotValidBeforeT}}, + Scripts: []transaction.Witness{ + { + InvocationScript: []byte{0x0D, 0x0E}, + VerificationScript: []byte{0x0F, 0x10}, + }, + }, + }, + Witness: transaction.Witness{ + InvocationScript: []byte{0x11, 0x12}, + VerificationScript: []byte{0x13, 0x14}, + }, + } + + p2pCopy := orig.Copy() + + require.Equal(t, orig, p2pCopy) + require.NotSame(t, orig, p2pCopy) + + require.Equal(t, orig.MainTransaction, p2pCopy.MainTransaction) + require.Equal(t, orig.FallbackTransaction, p2pCopy.FallbackTransaction) + require.Equal(t, orig.Witness, p2pCopy.Witness) + + p2pCopy.MainTransaction.Version = 3 + p2pCopy.FallbackTransaction.Script[0] = 0x1F + p2pCopy.Witness.VerificationScript[1] = 0x22 + + require.NotEqual(t, orig.MainTransaction.Version, p2pCopy.MainTransaction.Version) + require.NotEqual(t, orig.FallbackTransaction.Script[0], p2pCopy.FallbackTransaction.Script[0]) + require.NotEqual(t, orig.Witness.VerificationScript[1], p2pCopy.Witness.VerificationScript[1]) +} From 8da7c0a671c6b56ba84255682f0ba10ffea70b1e Mon Sep 17 00:00:00 2001 From: Ekaterina Pavlova Date: Tue, 23 Apr 2024 16:05:11 +0200 Subject: [PATCH 5/5] notary: reuse (*Transaction).Copy where possible Signed-off-by: Ekaterina Pavlova --- pkg/services/notary/notary.go | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/pkg/services/notary/notary.go b/pkg/services/notary/notary.go index 1f75f9e45..166890675 100644 --- a/pkg/services/notary/notary.go +++ b/pkg/services/notary/notary.go @@ -272,7 +272,7 @@ func (n *Notary) OnNewRequest(payload *payload.P2PNotaryRequest) { // Avoid changes in the main transaction witnesses got from the notary request pool to // keep the pooled tx valid. We will update its copy => the copy's size will be changed. r = &request{ - main: safeCopy(payload.MainTransaction), + main: payload.MainTransaction.Copy(), minNotValidBefore: nvbFallback, } n.requests[payload.MainTransaction.Hash()] = r @@ -285,7 +285,7 @@ func (n *Notary) OnNewRequest(payload *payload.P2PNotaryRequest) { // size won't be changed after finalisation, the witness bytes changes may // affect the other users of notary pool and cause race. Avoid this by making // the copy. - r.fallbacks = append(r.fallbacks, safeCopy(payload.FallbackTransaction)) + r.fallbacks = append(r.fallbacks, payload.FallbackTransaction.Copy()) if exists && r.isMainCompleted() || validationErr != nil { return } @@ -345,21 +345,6 @@ func (n *Notary) OnNewRequest(payload *payload.P2PNotaryRequest) { } } -// safeCopy creates a copy of provided transaction by dereferencing it and creating -// fresh witnesses so that the tx's witnesses may be modified without affecting the -// copy's ones. -func safeCopy(tx *transaction.Transaction) *transaction.Transaction { - cp := *tx - cp.Scripts = make([]transaction.Witness, len(tx.Scripts)) - for i := range cp.Scripts { - cp.Scripts[i] = transaction.Witness{ - InvocationScript: bytes.Clone(tx.Scripts[i].InvocationScript), - VerificationScript: bytes.Clone(tx.Scripts[i].VerificationScript), - } - } - return &cp -} - // OnRequestRemoval is a callback which is called after fallback transaction is removed // from the notary payload pool due to expiration, main tx appliance or any other reason. func (n *Notary) OnRequestRemoval(pld *payload.P2PNotaryRequest) {