From f3c1283ac67169f2f19ec0566ecf716dbbc004bc Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 7 Apr 2023 11:26:58 +0300 Subject: [PATCH 1/2] *: move NVB and Conflicts attributes out of extensions They're a part of the regular protocol now. Signed-off-by: Anna Shaleva Signed-off-by: Roman Khimov --- docs/node-configuration.md | 2 +- pkg/core/blockchain.go | 6 - pkg/core/blockchain_neotest_test.go | 10 +- pkg/core/dao/dao.go | 89 +++++++------- pkg/core/mempool/feer.go | 1 - pkg/core/mempool/mem_pool.go | 138 ++++++++++------------ pkg/core/mempool/mem_pool_test.go | 4 - pkg/network/notary_feer.go | 5 - pkg/network/server_test.go | 1 - pkg/services/rpcsrv/server.go | 1 + pkg/services/rpcsrv/server_helper_test.go | 4 - 11 files changed, 109 insertions(+), 152 deletions(-) diff --git a/docs/node-configuration.md b/docs/node-configuration.md index e7088c979..088e75e95 100644 --- a/docs/node-configuration.md +++ b/docs/node-configuration.md @@ -356,7 +356,7 @@ protocol-related settings described in the table below. | MemPoolSize | `int` | `50000` | Size of the node's memory pool where transactions are stored before they are added to block. | | NativeActivations | `map[string][]uint32` | ContractManagement: [0]
StdLib: [0]
CryptoLib: [0]
LedgerContract: [0]
NeoToken: [0]
GasToken: [0]
PolicyContract: [0]
RoleManagement: [0]
OracleContract: [0] | The list of histories of native contracts updates. Each list item shod be presented as a known native contract name with the corresponding list of chain's heights. The contract is not active until chain reaches the first height value specified in the list. | `Notary` is supported. | | P2PNotaryRequestPayloadPoolSize | `int` | `1000` | Size of the node's P2P Notary request payloads memory pool where P2P Notary requests are stored before main or fallback transaction is completed and added to the chain.
This option is valid only if `P2PSigExtensions` are enabled. | Not supported by the C# node, thus may affect heterogeneous networks functionality. | -| P2PSigExtensions | `bool` | `false` | Enables following additional Notary service related logic:
• Transaction attributes `NotValidBefore`, `Conflicts` and `NotaryAssisted`
• Network payload of the `P2PNotaryRequest` type
• Native `Notary` contract
• Notary node module | Not supported by the C# node, thus may affect heterogeneous networks functionality. | +| P2PSigExtensions | `bool` | `false` | Enables following additional Notary service related logic:
• Transaction attribute `NotaryAssisted`
• Network payload of the `P2PNotaryRequest` type
• Native `Notary` contract
• Notary node module | Not supported by the C# node, thus may affect heterogeneous networks functionality. | | P2PStateExchangeExtensions | `bool` | `false` | Enables the following P2P MPT state data exchange logic:
• `StateSyncInterval` protocol setting
• P2P commands `GetMPTDataCMD` and `MPTDataCMD` | Not supported by the C# node, thus may affect heterogeneous networks functionality. Can be supported either on MPT-complete node (`KeepOnlyLatestState`=`false`) or on light GC-enabled node (`RemoveUntraceableBlocks=true`) in which case `KeepOnlyLatestState` setting doesn't change the behavior, an appropriate set of MPTs is always stored (see `RemoveUntraceableBlocks`). | | RemoveUntraceableBlocks | `bool`| `false` | Denotes whether old blocks should be removed from cache and database. If enabled, then only the last `MaxTraceableBlocks` are stored and accessible to smart contracts. Old MPT data is also deleted in accordance with `GarbageCollectionPeriod` setting. If enabled along with `P2PStateExchangeExtensions`, then old blocks and MPT states will be removed up to the second latest state synchronisation point (see `StateSyncInterval`). | This setting is deprecated in favor of the same setting in the ApplicationConfiguration and will be removed in future node versions. If both settings are used, setting any of them to true enables the function. | | ReservedAttributes | `bool` | `false` | Allows to have reserved attributes range for experimental or private purposes. | diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 74f63a110..974d9ffa5 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -2573,9 +2573,6 @@ func (bc *Blockchain) verifyTxAttributes(d *dao.Simple, tx *transaction.Transact return fmt.Errorf("%w: oracle tx has insufficient gas", ErrInvalidAttribute) } case transaction.NotValidBeforeT: - if !bc.config.P2PSigExtensions { - return fmt.Errorf("%w: NotValidBefore attribute was found, but P2PSigExtensions are disabled", ErrInvalidAttribute) - } nvb := tx.Attributes[i].Value.(*transaction.NotValidBefore).Height curHeight := bc.BlockHeight() if isPartialTx { @@ -2595,9 +2592,6 @@ func (bc *Blockchain) verifyTxAttributes(d *dao.Simple, tx *transaction.Transact } } case transaction.ConflictsT: - if !bc.config.P2PSigExtensions { - return fmt.Errorf("%w: Conflicts attribute was found, but P2PSigExtensions are disabled", ErrInvalidAttribute) - } conflicts := tx.Attributes[i].Value.(*transaction.Conflicts) // Only fully-qualified dao.ErrAlreadyExists error bothers us here, thus, we // can safely omit the payer argument to HasTransaction call to improve performance a bit. diff --git a/pkg/core/blockchain_neotest_test.go b/pkg/core/blockchain_neotest_test.go index 53480424a..959d3ed76 100644 --- a/pkg/core/blockchain_neotest_test.go +++ b/pkg/core/blockchain_neotest_test.go @@ -1535,7 +1535,7 @@ func TestBlockchain_VerifyTx(t *testing.T) { }} return tx } - t.Run("Disabled", func(t *testing.T) { + t.Run("Disabled", func(t *testing.T) { // check that NVB attribute is not an extension anymore. bcBad, validatorBad, committeeBad := chain.NewMultiWithCustomConfig(t, func(c *config.Blockchain) { c.P2PSigExtensions = false c.ReservedAttributes = false @@ -1543,8 +1543,7 @@ func TestBlockchain_VerifyTx(t *testing.T) { eBad := neotest.NewExecutor(t, bcBad, validatorBad, committeeBad) tx := getNVBTx(eBad, bcBad.BlockHeight()) err := bcBad.VerifyTx(tx) - require.Error(t, err) - require.True(t, strings.Contains(err.Error(), "invalid attribute: NotValidBefore attribute was found, but P2PSigExtensions are disabled")) + require.NoError(t, err) }) t.Run("Enabled", func(t *testing.T) { t.Run("NotYetValid", func(t *testing.T) { @@ -1621,7 +1620,7 @@ func TestBlockchain_VerifyTx(t *testing.T) { }} return tx } - t.Run("disabled", func(t *testing.T) { + t.Run("disabled", func(t *testing.T) { // check that Conflicts attribute is not an extension anymore. bcBad, validatorBad, committeeBad := chain.NewMultiWithCustomConfig(t, func(c *config.Blockchain) { c.P2PSigExtensions = false c.ReservedAttributes = false @@ -1629,8 +1628,7 @@ func TestBlockchain_VerifyTx(t *testing.T) { eBad := neotest.NewExecutor(t, bcBad, validatorBad, committeeBad) tx := getConflictsTx(eBad, util.Uint256{1, 2, 3}) err := bcBad.VerifyTx(tx) - require.Error(t, err) - require.True(t, strings.Contains(err.Error(), "invalid attribute: Conflicts attribute was found, but P2PSigExtensions are disabled")) + require.NoError(t, err) }) t.Run("enabled", func(t *testing.T) { t.Run("dummy on-chain conflict", func(t *testing.T) { diff --git a/pkg/core/dao/dao.go b/pkg/core/dao/dao.go index b6092e1fc..c6f08ccbd 100644 --- a/pkg/core/dao/dao.go +++ b/pkg/core/dao/dao.go @@ -766,12 +766,10 @@ func (dao *Simple) DeleteBlock(h util.Uint256) error { for _, tx := range b.Transactions { copy(key[1:], tx.Hash().BytesBE()) dao.Store.Delete(key) - if dao.Version.P2PSigExtensions { - for _, attr := range tx.GetAttributes(transaction.ConflictsT) { - hash := attr.Value.(*transaction.Conflicts).Hash - copy(key[1:], hash.BytesBE()) - dao.Store.Delete(key) - } + for _, attr := range tx.GetAttributes(transaction.ConflictsT) { + hash := attr.Value.(*transaction.Conflicts).Hash + copy(key[1:], hash.BytesBE()) + dao.Store.Delete(key) } } @@ -830,51 +828,50 @@ func (dao *Simple) StoreAsTransaction(tx *transaction.Transaction, index uint32, return buf.Err } dao.Store.Put(key, buf.Bytes()) - if dao.Version.P2PSigExtensions { - var ( - valuePrefix []byte - newSigners []byte - ) - attrs := tx.GetAttributes(transaction.ConflictsT) - for _, attr := range attrs { - hash := attr.Value.(*transaction.Conflicts).Hash - copy(key[1:], hash.BytesBE()) - old, err := dao.Store.Get(key) - if err != nil && !errors.Is(err, storage.ErrKeyNotFound) { - return fmt.Errorf("failed to retrieve previous conflict record for %s: %w", hash.StringLE(), err) + var ( + valuePrefix []byte + newSigners []byte + ) + attrs := tx.GetAttributes(transaction.ConflictsT) + for _, attr := range attrs { + hash := attr.Value.(*transaction.Conflicts).Hash + copy(key[1:], hash.BytesBE()) + + old, err := dao.Store.Get(key) + if err != nil && !errors.Is(err, storage.ErrKeyNotFound) { + return fmt.Errorf("failed to retrieve previous conflict record for %s: %w", hash.StringLE(), err) + } + if err == nil { + if len(old) <= 6 { // storage.ExecTransaction + U32LE index + transaction.DummyVersion + return fmt.Errorf("invalid conflict record format of length %d", len(old)) } - if err == nil { - if len(old) <= 6 { // storage.ExecTransaction + U32LE index + transaction.DummyVersion - return fmt.Errorf("invalid conflict record format of length %d", len(old)) - } - } - buf.Reset() - buf.WriteBytes(old) - if len(old) == 0 { - if len(valuePrefix) != 0 { - buf.WriteBytes(valuePrefix) - } else { - buf.WriteB(storage.ExecTransaction) - buf.WriteU32LE(index) - buf.WriteB(transaction.DummyVersion) - } - } - newSignersOffset := buf.Len() - if len(newSigners) == 0 { - for _, s := range tx.Signers { - s.Account.EncodeBinary(buf.BinWriter) - } + } + buf.Reset() + buf.WriteBytes(old) + if len(old) == 0 { + if len(valuePrefix) != 0 { + buf.WriteBytes(valuePrefix) } else { - buf.WriteBytes(newSigners) + buf.WriteB(storage.ExecTransaction) + buf.WriteU32LE(index) + buf.WriteB(transaction.DummyVersion) } - val := buf.Bytes() - dao.Store.Put(key, val) + } + newSignersOffset := buf.Len() + if len(newSigners) == 0 { + for _, s := range tx.Signers { + s.Account.EncodeBinary(buf.BinWriter) + } + } else { + buf.WriteBytes(newSigners) + } + val := buf.Bytes() + dao.Store.Put(key, val) - if len(attrs) > 1 && len(valuePrefix) == 0 { - valuePrefix = slice.Copy(val[:6]) - newSigners = slice.Copy(val[newSignersOffset:]) - } + if len(attrs) > 1 && len(valuePrefix) == 0 { + valuePrefix = slice.Copy(val[:6]) + newSigners = slice.Copy(val[newSignersOffset:]) } } return nil diff --git a/pkg/core/mempool/feer.go b/pkg/core/mempool/feer.go index 55efd4eef..67ddfb812 100644 --- a/pkg/core/mempool/feer.go +++ b/pkg/core/mempool/feer.go @@ -11,5 +11,4 @@ type Feer interface { FeePerByte() int64 GetUtilityTokenBalance(util.Uint160) *big.Int BlockHeight() uint32 - P2PSigExtensionsEnabled() bool } diff --git a/pkg/core/mempool/mem_pool.go b/pkg/core/mempool/mem_pool.go index b03c1f750..5d4c3a007 100644 --- a/pkg/core/mempool/mem_pool.go +++ b/pkg/core/mempool/mem_pool.go @@ -145,16 +145,14 @@ func (mp *Pool) HasConflicts(t *transaction.Transaction, fee Feer) bool { if mp.containsKey(t.Hash()) { return true } - if fee.P2PSigExtensionsEnabled() { - // do not check sender's signature and fee - if _, ok := mp.conflicts[t.Hash()]; ok { + // do not check sender's signature and fee + if _, ok := mp.conflicts[t.Hash()]; ok { + return true + } + for _, attr := range t.GetAttributes(transaction.ConflictsT) { + if mp.containsKey(attr.Value.(*transaction.Conflicts).Hash) { return true } - for _, attr := range t.GetAttributes(transaction.ConflictsT) { - if mp.containsKey(attr.Value.(*transaction.Conflicts).Hash) { - return true - } - } } return false } @@ -229,11 +227,9 @@ func (mp *Pool) Add(t *transaction.Transaction, fee Feer, data ...any) error { mp.oracleResp[id] = t.Hash() } - if fee.P2PSigExtensionsEnabled() { - // Remove conflicting transactions. - for _, conflictingTx := range conflictsToBeRemoved { - mp.removeInternal(conflictingTx.Hash(), fee) - } + // Remove conflicting transactions. + for _, conflictingTx := range conflictsToBeRemoved { + mp.removeInternal(conflictingTx.Hash(), fee) } // Insert into a sorted array (from max to min, that could also be done // using sort.Sort(sort.Reverse()), but it incurs more overhead. Notice @@ -255,9 +251,7 @@ func (mp *Pool) Add(t *transaction.Transaction, fee Feer, data ...any) error { // Ditch the last one. unlucky := mp.verifiedTxes[len(mp.verifiedTxes)-1] delete(mp.verifiedMap, unlucky.txn.Hash()) - if fee.P2PSigExtensionsEnabled() { - mp.removeConflictsOf(unlucky.txn) - } + mp.removeConflictsOf(unlucky.txn) if attrs := unlucky.txn.GetAttributes(transaction.OracleResponseT); len(attrs) != 0 { delete(mp.oracleResp, attrs[0].Value.(*transaction.OracleResponse).ID) } @@ -277,12 +271,10 @@ func (mp *Pool) Add(t *transaction.Transaction, fee Feer, data ...any) error { mp.verifiedTxes[n] = pItem } mp.verifiedMap[t.Hash()] = t - if fee.P2PSigExtensionsEnabled() { - // Add conflicting hashes to the mp.conflicts list. - for _, attr := range t.GetAttributes(transaction.ConflictsT) { - hash := attr.Value.(*transaction.Conflicts).Hash - mp.conflicts[hash] = append(mp.conflicts[hash], t.Hash()) - } + // Add conflicting hashes to the mp.conflicts list. + for _, attr := range t.GetAttributes(transaction.ConflictsT) { + hash := attr.Value.(*transaction.Conflicts).Hash + mp.conflicts[hash] = append(mp.conflicts[hash], t.Hash()) } // we already checked balance in checkTxConflicts, so don't need to check again mp.tryAddSendersFee(pItem.txn, fee, false) @@ -330,10 +322,8 @@ func (mp *Pool) removeInternal(hash util.Uint256, feer Feer) { senderFee := mp.fees[payer] senderFee.feeSum.SubUint64(&senderFee.feeSum, uint64(tx.SystemFee+tx.NetworkFee)) mp.fees[payer] = senderFee - if feer.P2PSigExtensionsEnabled() { - // remove all conflicting hashes from mp.conflicts list - mp.removeConflictsOf(tx) - } + // remove all conflicting hashes from mp.conflicts list + mp.removeConflictsOf(tx) if attrs := tx.GetAttributes(transaction.OracleResponseT); len(attrs) != 0 { delete(mp.oracleResp, attrs[0].Value.(*transaction.OracleResponse).ID) } @@ -360,9 +350,7 @@ func (mp *Pool) RemoveStale(isOK func(*transaction.Transaction) bool, feer Feer) // because items are iterated one-by-one in increasing order. newVerifiedTxes := mp.verifiedTxes[:0] mp.fees = make(map[util.Uint160]utilityBalanceAndFees) // it'd be nice to reuse existing map, but we can't easily clear it - if feer.P2PSigExtensionsEnabled() { - mp.conflicts = make(map[util.Uint256][]util.Uint256) - } + mp.conflicts = make(map[util.Uint256][]util.Uint256) height := feer.BlockHeight() var ( staleItems []item @@ -370,11 +358,9 @@ func (mp *Pool) RemoveStale(isOK func(*transaction.Transaction) bool, feer Feer) for _, itm := range mp.verifiedTxes { if isOK(itm.txn) && mp.checkPolicy(itm.txn, policyChanged) && mp.tryAddSendersFee(itm.txn, feer, true) { newVerifiedTxes = append(newVerifiedTxes, itm) - if feer.P2PSigExtensionsEnabled() { - for _, attr := range itm.txn.GetAttributes(transaction.ConflictsT) { - hash := attr.Value.(*transaction.Conflicts).Hash - mp.conflicts[hash] = append(mp.conflicts[hash], itm.txn.Hash()) - } + for _, attr := range itm.txn.GetAttributes(transaction.ConflictsT) { + hash := attr.Value.(*transaction.Conflicts).Hash + mp.conflicts[hash] = append(mp.conflicts[hash], itm.txn.Hash()) } if mp.resendThreshold != 0 { // item is resent at resendThreshold, 2*resendThreshold, 4*resendThreshold ... @@ -524,56 +510,52 @@ func (mp *Pool) checkTxConflicts(tx *transaction.Transaction, fee Feer) ([]*tran conflictsToBeRemoved []*transaction.Transaction conflictingFee int64 ) - if fee.P2PSigExtensionsEnabled() { - // Step 1: check if `tx` was in attributes of mempooled transactions. - if conflictingHashes, ok := mp.conflicts[tx.Hash()]; ok { - for _, hash := range conflictingHashes { - existingTx := mp.verifiedMap[hash] - if existingTx.HasSigner(payer) { - conflictingFee += existingTx.NetworkFee - } - conflictsToBeRemoved = append(conflictsToBeRemoved, existingTx) - } - } - // Step 2: check if mempooled transactions were in `tx`'s attributes. - conflictsAttrs := tx.GetAttributes(transaction.ConflictsT) - if len(conflictsAttrs) != 0 { - txSigners := make(map[util.Uint160]struct{}, len(tx.Signers)) - for _, s := range tx.Signers { - txSigners[s.Account] = struct{}{} - } - for _, attr := range conflictsAttrs { - hash := attr.Value.(*transaction.Conflicts).Hash - existingTx, ok := mp.verifiedMap[hash] - if !ok { - continue - } - var signerOK bool - for _, s := range existingTx.Signers { - if _, ok := txSigners[s.Account]; ok { - signerOK = true - break - } - } - if !signerOK { - return nil, fmt.Errorf("%w: not signed by a signer of conflicting transaction %s", ErrConflictsAttribute, existingTx.Hash().StringBE()) - } + // Step 1: check if `tx` was in attributes of mempooled transactions. + if conflictingHashes, ok := mp.conflicts[tx.Hash()]; ok { + for _, hash := range conflictingHashes { + existingTx := mp.verifiedMap[hash] + if existingTx.HasSigner(payer) { conflictingFee += existingTx.NetworkFee - conflictsToBeRemoved = append(conflictsToBeRemoved, existingTx) } + conflictsToBeRemoved = append(conflictsToBeRemoved, existingTx) } - if conflictingFee != 0 && tx.NetworkFee <= conflictingFee { - return nil, fmt.Errorf("%w: conflicting transactions have bigger or equal network fee: %d vs %d", ErrConflictsAttribute, tx.NetworkFee, conflictingFee) + } + // Step 2: check if mempooled transactions were in `tx`'s attributes. + conflictsAttrs := tx.GetAttributes(transaction.ConflictsT) + if len(conflictsAttrs) != 0 { + txSigners := make(map[util.Uint160]struct{}, len(tx.Signers)) + for _, s := range tx.Signers { + txSigners[s.Account] = struct{}{} } - // Step 3: take into account sender's conflicting transactions before balance check. - expectedSenderFee = actualSenderFee - for _, conflictingTx := range conflictsToBeRemoved { - if conflictingTx.Signers[mp.payerIndex].Account.Equals(payer) { - expectedSenderFee.feeSum.SubUint64(&expectedSenderFee.feeSum, uint64(conflictingTx.SystemFee+conflictingTx.NetworkFee)) + for _, attr := range conflictsAttrs { + hash := attr.Value.(*transaction.Conflicts).Hash + existingTx, ok := mp.verifiedMap[hash] + if !ok { + continue } + var signerOK bool + for _, s := range existingTx.Signers { + if _, ok := txSigners[s.Account]; ok { + signerOK = true + break + } + } + if !signerOK { + return nil, fmt.Errorf("%w: not signed by a signer of conflicting transaction %s", ErrConflictsAttribute, existingTx.Hash().StringBE()) + } + conflictingFee += existingTx.NetworkFee + conflictsToBeRemoved = append(conflictsToBeRemoved, existingTx) + } + } + if conflictingFee != 0 && tx.NetworkFee <= conflictingFee { + return nil, fmt.Errorf("%w: conflicting transactions have bigger or equal network fee: %d vs %d", ErrConflictsAttribute, tx.NetworkFee, conflictingFee) + } + // Step 3: take into account sender's conflicting transactions before balance check. + expectedSenderFee = actualSenderFee + for _, conflictingTx := range conflictsToBeRemoved { + if conflictingTx.Signers[mp.payerIndex].Account.Equals(payer) { + expectedSenderFee.feeSum.SubUint64(&expectedSenderFee.feeSum, uint64(conflictingTx.SystemFee+conflictingTx.NetworkFee)) } - } else { - expectedSenderFee = actualSenderFee } _, err := checkBalance(tx, expectedSenderFee) return conflictsToBeRemoved, err diff --git a/pkg/core/mempool/mem_pool_test.go b/pkg/core/mempool/mem_pool_test.go index 9bcf43cba..430f4ea05 100644 --- a/pkg/core/mempool/mem_pool_test.go +++ b/pkg/core/mempool/mem_pool_test.go @@ -40,10 +40,6 @@ func (fs *FeerStub) GetUtilityTokenBalance(uint160 util.Uint160) *big.Int { return big.NewInt(fs.balance) } -func (fs *FeerStub) P2PSigExtensionsEnabled() bool { - return fs.p2pSigExt -} - func testMemPoolAddRemoveWithFeer(t *testing.T, fs Feer) { mp := New(10, 0, false, nil) tx := transaction.New([]byte{byte(opcode.PUSH1)}, 0) diff --git a/pkg/network/notary_feer.go b/pkg/network/notary_feer.go index 1fbc82111..2f470006e 100644 --- a/pkg/network/notary_feer.go +++ b/pkg/network/notary_feer.go @@ -26,11 +26,6 @@ func (f NotaryFeer) BlockHeight() uint32 { return f.bc.BlockHeight() } -// P2PSigExtensionsEnabled implements mempool.Feer interface. -func (f NotaryFeer) P2PSigExtensionsEnabled() bool { - return f.bc.P2PSigExtensionsEnabled() -} - // NewNotaryFeer returns new NotaryFeer instance. func NewNotaryFeer(bc Ledger) NotaryFeer { return NotaryFeer{ diff --git a/pkg/network/server_test.go b/pkg/network/server_test.go index 1f862d105..72e5f156c 100644 --- a/pkg/network/server_test.go +++ b/pkg/network/server_test.go @@ -1001,7 +1001,6 @@ type feerStub struct { func (f feerStub) FeePerByte() int64 { return 1 } func (f feerStub) GetUtilityTokenBalance(util.Uint160) *big.Int { return big.NewInt(100000000) } func (f feerStub) BlockHeight() uint32 { return f.blockHeight } -func (f feerStub) P2PSigExtensionsEnabled() bool { return false } func (f feerStub) GetBaseExecFee() int64 { return interop.DefaultBaseExecFee } func TestMemPool(t *testing.T) { diff --git a/pkg/services/rpcsrv/server.go b/pkg/services/rpcsrv/server.go index 5e66e0e3a..f973d0489 100644 --- a/pkg/services/rpcsrv/server.go +++ b/pkg/services/rpcsrv/server.go @@ -98,6 +98,7 @@ type ( GetTransaction(util.Uint256) (*transaction.Transaction, uint32, error) HeaderHeight() uint32 InitVerificationContext(ic *interop.Context, hash util.Uint160, witness *transaction.Witness) error + P2PSigExtensionsEnabled() bool SubscribeForBlocks(ch chan *block.Block) SubscribeForExecutions(ch chan *state.AppExecResult) SubscribeForNotifications(ch chan *state.ContainedNotificationEvent) diff --git a/pkg/services/rpcsrv/server_helper_test.go b/pkg/services/rpcsrv/server_helper_test.go index 893ef1dcb..e0a1ef68c 100644 --- a/pkg/services/rpcsrv/server_helper_test.go +++ b/pkg/services/rpcsrv/server_helper_test.go @@ -173,10 +173,6 @@ func (fs *FeerStub) GetUtilityTokenBalance(acc util.Uint160) *big.Int { return big.NewInt(1000000 * native.GASFactor) } -func (fs FeerStub) P2PSigExtensionsEnabled() bool { - return false -} - func (fs FeerStub) GetBaseExecFee() int64 { return interop.DefaultBaseExecFee } From fff7e9170989a5f602572128e1c7b1986bb1a8e5 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 4 Sep 2023 16:48:16 +0300 Subject: [PATCH 2/2] dao: simplify NewSimple() We no longer need P2PSigExtension flag here, conflicts attribute is a part of the normal protocol. Signed-off-by: Roman Khimov --- pkg/compiler/interop_test.go | 2 +- pkg/core/blockchain.go | 6 ++--- pkg/core/blockchain_neotest_test.go | 2 +- pkg/core/dao/dao.go | 9 +++---- pkg/core/dao/dao_test.go | 38 +++++++++++++-------------- pkg/core/interop/crypto/ecdsa_test.go | 4 +-- pkg/core/native/management_test.go | 8 +++--- pkg/core/statesync/module_test.go | 2 +- 8 files changed, 35 insertions(+), 36 deletions(-) diff --git a/pkg/compiler/interop_test.go b/pkg/compiler/interop_test.go index f27c1bd6b..875fb56fb 100644 --- a/pkg/compiler/interop_test.go +++ b/pkg/compiler/interop_test.go @@ -320,7 +320,7 @@ func TestAppCall(t *testing.T) { } fc := fakechain.NewFakeChain() - ic := interop.NewContext(trigger.Application, fc, dao.NewSimple(storage.NewMemoryStore(), false, false), + ic := interop.NewContext(trigger.Application, fc, dao.NewSimple(storage.NewMemoryStore(), false), interop.DefaultBaseExecFee, native.DefaultStoragePrice, contractGetter, nil, nil, nil, nil, zaptest.NewLogger(t)) t.Run("valid script", func(t *testing.T) { diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 974d9ffa5..c2f3e9e00 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -320,8 +320,8 @@ func NewBlockchain(s storage.Store, cfg config.Blockchain, log *zap.Logger) (*Bl } bc := &Blockchain{ config: cfg, - dao: dao.NewSimple(s, cfg.StateRootInHeader, cfg.P2PSigExtensions), - persistent: dao.NewSimple(s, cfg.StateRootInHeader, cfg.P2PSigExtensions), + dao: dao.NewSimple(s, cfg.StateRootInHeader), + persistent: dao.NewSimple(s, cfg.StateRootInHeader), store: s, stopCh: make(chan struct{}), runToExitCh: make(chan struct{}), @@ -2752,7 +2752,7 @@ func (bc *Blockchain) GetTestHistoricVM(t trigger.Type, tx *transaction.Transact return nil, fmt.Errorf("failed to retrieve stateroot for height %d: %w", b.Index, err) } s := mpt.NewTrieStore(sr.Root, mode, storage.NewPrivateMemCachedStore(bc.dao.Store)) - dTrie := dao.NewSimple(s, bc.config.StateRootInHeader, bc.config.P2PSigExtensions) + dTrie := dao.NewSimple(s, bc.config.StateRootInHeader) dTrie.Version = bc.dao.Version // Initialize native cache before passing DAO to interop context constructor, because // the constructor will call BaseExecFee/StoragePrice policy methods on the passed DAO. diff --git a/pkg/core/blockchain_neotest_test.go b/pkg/core/blockchain_neotest_test.go index 959d3ed76..f26eb777d 100644 --- a/pkg/core/blockchain_neotest_test.go +++ b/pkg/core/blockchain_neotest_test.go @@ -94,7 +94,7 @@ func TestBlockchain_StartFromExistingDB(t *testing.T) { t.Run("mismatch storage version", func(t *testing.T) { ps = newPS(t) cache := storage.NewMemCachedStore(ps) // Extra wrapper to avoid good DB corruption. - d := dao.NewSimple(cache, bc.GetConfig().StateRootInHeader, bc.GetConfig().P2PStateExchangeExtensions) + d := dao.NewSimple(cache, bc.GetConfig().StateRootInHeader) d.PutVersion(dao.Version{ Value: "0.0.0", }) diff --git a/pkg/core/dao/dao.go b/pkg/core/dao/dao.go index c6f08ccbd..03d5b0f37 100644 --- a/pkg/core/dao/dao.go +++ b/pkg/core/dao/dao.go @@ -62,17 +62,16 @@ type NativeContractCache interface { } // NewSimple creates a new simple dao using the provided backend store. -func NewSimple(backend storage.Store, stateRootInHeader bool, p2pSigExtensions bool) *Simple { +func NewSimple(backend storage.Store, stateRootInHeader bool) *Simple { st := storage.NewMemCachedStore(backend) - return newSimple(st, stateRootInHeader, p2pSigExtensions) + return newSimple(st, stateRootInHeader) } -func newSimple(st *storage.MemCachedStore, stateRootInHeader bool, p2pSigExtensions bool) *Simple { +func newSimple(st *storage.MemCachedStore, stateRootInHeader bool) *Simple { return &Simple{ Version: Version{ StoragePrefix: storage.STStorage, StateRootInHeader: stateRootInHeader, - P2PSigExtensions: p2pSigExtensions, }, Store: st, nativeCache: make(map[int32]NativeContractCache), @@ -87,7 +86,7 @@ func (dao *Simple) GetBatch() *storage.MemBatch { // GetWrapped returns a new DAO instance with another layer of wrapped // MemCachedStore around the current DAO Store. func (dao *Simple) GetWrapped() *Simple { - d := NewSimple(dao.Store, dao.Version.StateRootInHeader, dao.Version.P2PSigExtensions) + d := NewSimple(dao.Store, dao.Version.StateRootInHeader) d.Version = dao.Version d.nativeCachePS = dao return d diff --git a/pkg/core/dao/dao_test.go b/pkg/core/dao/dao_test.go index 1e8b647cd..6e83e2218 100644 --- a/pkg/core/dao/dao_test.go +++ b/pkg/core/dao/dao_test.go @@ -18,7 +18,7 @@ import ( ) func TestPutGetAndDecode(t *testing.T) { - dao := NewSimple(storage.NewMemoryStore(), false, false) + dao := NewSimple(storage.NewMemoryStore(), false) serializable := &TestSerializable{field: random.String(4)} hash := []byte{1} require.NoError(t, dao.putWithBuffer(serializable, hash, io.NewBufBinWriter())) @@ -42,7 +42,7 @@ func (t *TestSerializable) DecodeBinary(reader *io.BinReader) { } func TestPutGetStorageItem(t *testing.T) { - dao := NewSimple(storage.NewMemoryStore(), false, false) + dao := NewSimple(storage.NewMemoryStore(), false) id := int32(random.Int(0, 1024)) key := []byte{0} storageItem := state.StorageItem{} @@ -52,7 +52,7 @@ func TestPutGetStorageItem(t *testing.T) { } func TestDeleteStorageItem(t *testing.T) { - dao := NewSimple(storage.NewMemoryStore(), false, false) + dao := NewSimple(storage.NewMemoryStore(), false) id := int32(random.Int(0, 1024)) key := []byte{0} storageItem := state.StorageItem{} @@ -63,7 +63,7 @@ func TestDeleteStorageItem(t *testing.T) { } func TestGetBlock_NotExists(t *testing.T) { - dao := NewSimple(storage.NewMemoryStore(), false, false) + dao := NewSimple(storage.NewMemoryStore(), false) hash := random.Uint256() block, err := dao.GetBlock(hash) require.Error(t, err) @@ -71,7 +71,7 @@ func TestGetBlock_NotExists(t *testing.T) { } func TestPutGetBlock(t *testing.T) { - dao := NewSimple(storage.NewMemoryStore(), false, false) + dao := NewSimple(storage.NewMemoryStore(), false) b := &block.Block{ Header: block.Header{ Script: transaction.Witness{ @@ -110,14 +110,14 @@ func TestPutGetBlock(t *testing.T) { } func TestGetVersion_NoVersion(t *testing.T) { - dao := NewSimple(storage.NewMemoryStore(), false, false) + dao := NewSimple(storage.NewMemoryStore(), false) version, err := dao.GetVersion() require.Error(t, err) require.Equal(t, "", version.Value) } func TestGetVersion(t *testing.T) { - dao := NewSimple(storage.NewMemoryStore(), false, false) + dao := NewSimple(storage.NewMemoryStore(), false) expected := Version{ StoragePrefix: 0x42, P2PSigExtensions: true, @@ -130,14 +130,14 @@ func TestGetVersion(t *testing.T) { require.Equal(t, expected, actual) t.Run("invalid", func(t *testing.T) { - dao := NewSimple(storage.NewMemoryStore(), false, false) + dao := NewSimple(storage.NewMemoryStore(), false) dao.Store.Put([]byte{byte(storage.SYSVersion)}, []byte("0.1.2\x00x")) _, err := dao.GetVersion() require.Error(t, err) }) t.Run("old format", func(t *testing.T) { - dao := NewSimple(storage.NewMemoryStore(), false, false) + dao := NewSimple(storage.NewMemoryStore(), false) dao.Store.Put([]byte{byte(storage.SYSVersion)}, []byte("0.1.2")) version, err := dao.GetVersion() @@ -147,14 +147,14 @@ func TestGetVersion(t *testing.T) { } func TestGetCurrentHeaderHeight_NoHeader(t *testing.T) { - dao := NewSimple(storage.NewMemoryStore(), false, false) + dao := NewSimple(storage.NewMemoryStore(), false) height, err := dao.GetCurrentBlockHeight() require.Error(t, err) require.Equal(t, uint32(0), height) } func TestGetCurrentHeaderHeight_Store(t *testing.T) { - dao := NewSimple(storage.NewMemoryStore(), false, false) + dao := NewSimple(storage.NewMemoryStore(), false) b := &block.Block{ Header: block.Header{ Script: transaction.Witness{ @@ -170,8 +170,8 @@ func TestGetCurrentHeaderHeight_Store(t *testing.T) { } func TestStoreAsTransaction(t *testing.T) { - t.Run("P2PSigExtensions off", func(t *testing.T) { - dao := NewSimple(storage.NewMemoryStore(), false, false) + t.Run("no conflicts", func(t *testing.T) { + dao := NewSimple(storage.NewMemoryStore(), false) tx := transaction.New([]byte{byte(opcode.PUSH1)}, 1) tx.Signers = append(tx.Signers, transaction.Signer{}) tx.Scripts = append(tx.Scripts, transaction.Witness{}) @@ -194,8 +194,8 @@ func TestStoreAsTransaction(t *testing.T) { require.Equal(t, *aer, gotAppExecResult[0]) }) - t.Run("P2PSigExtensions on", func(t *testing.T) { - dao := NewSimple(storage.NewMemoryStore(), false, true) + t.Run("with conflicts", func(t *testing.T) { + dao := NewSimple(storage.NewMemoryStore(), false) conflictsH := util.Uint256{1, 2, 3} signer1 := util.Uint160{1, 2, 3} signer2 := util.Uint160{4, 5, 6} @@ -279,7 +279,7 @@ func TestStoreAsTransaction(t *testing.T) { } func BenchmarkStoreAsTransaction(b *testing.B) { - dao := NewSimple(storage.NewMemoryStore(), false, true) + dao := NewSimple(storage.NewMemoryStore(), false) tx := transaction.New([]byte{byte(opcode.PUSH1)}, 1) tx.Attributes = []transaction.Attribute{ { @@ -324,7 +324,7 @@ func BenchmarkStoreAsTransaction(b *testing.B) { func TestMakeStorageItemKey(t *testing.T) { var id int32 = 5 - dao := NewSimple(storage.NewMemoryStore(), true, false) + dao := NewSimple(storage.NewMemoryStore(), true) expected := []byte{byte(storage.STStorage), 0, 0, 0, 0, 1, 2, 3} binary.LittleEndian.PutUint32(expected[1:5], uint32(id)) @@ -343,7 +343,7 @@ func TestMakeStorageItemKey(t *testing.T) { } func TestPutGetStateSyncPoint(t *testing.T) { - dao := NewSimple(storage.NewMemoryStore(), true, false) + dao := NewSimple(storage.NewMemoryStore(), true) // empty store _, err := dao.GetStateSyncPoint() @@ -358,7 +358,7 @@ func TestPutGetStateSyncPoint(t *testing.T) { } func TestPutGetStateSyncCurrentBlockHeight(t *testing.T) { - dao := NewSimple(storage.NewMemoryStore(), true, false) + dao := NewSimple(storage.NewMemoryStore(), true) // empty store _, err := dao.GetStateSyncCurrentBlockHeight() diff --git a/pkg/core/interop/crypto/ecdsa_test.go b/pkg/core/interop/crypto/ecdsa_test.go index a44c959ce..d789636b7 100644 --- a/pkg/core/interop/crypto/ecdsa_test.go +++ b/pkg/core/interop/crypto/ecdsa_test.go @@ -72,7 +72,7 @@ func initCheckMultisigVMNoArgs(container *transaction.Transaction) *vm.VM { ic := interop.NewContext( trigger.Verification, fakechain.NewFakeChain(), - dao.NewSimple(storage.NewMemoryStore(), false, false), + dao.NewSimple(storage.NewMemoryStore(), false), interop.DefaultBaseExecFee, native.DefaultStoragePrice, nil, nil, nil, nil, container, nil) @@ -178,7 +178,7 @@ func TestCheckSig(t *testing.T) { require.NoError(t, err) verifyFunc := ECDSASecp256r1CheckSig - d := dao.NewSimple(storage.NewMemoryStore(), false, false) + d := dao.NewSimple(storage.NewMemoryStore(), false) ic := &interop.Context{Network: uint32(netmode.UnitTestNet), DAO: d} runCase := func(t *testing.T, isErr bool, result any, args ...any) { ic.SpawnVM() diff --git a/pkg/core/native/management_test.go b/pkg/core/native/management_test.go index 657622e21..421899f24 100644 --- a/pkg/core/native/management_test.go +++ b/pkg/core/native/management_test.go @@ -18,7 +18,7 @@ import ( func TestDeployGetUpdateDestroyContract(t *testing.T) { mgmt := newManagement() mgmt.Policy = newPolicy() - d := dao.NewSimple(storage.NewMemoryStore(), false, false) + d := dao.NewSimple(storage.NewMemoryStore(), false) ic := &interop.Context{DAO: d} err := mgmt.Initialize(ic) require.NoError(t, err) @@ -81,13 +81,13 @@ func TestDeployGetUpdateDestroyContract(t *testing.T) { func TestManagement_Initialize(t *testing.T) { t.Run("good", func(t *testing.T) { - d := dao.NewSimple(storage.NewMemoryStore(), false, false) + d := dao.NewSimple(storage.NewMemoryStore(), false) mgmt := newManagement() require.NoError(t, mgmt.InitializeCache(0, d)) }) /* See #2801 t.Run("invalid contract state", func(t *testing.T) { - d := dao.NewSimple(storage.NewMemoryStore(), false, false) + d := dao.NewSimple(storage.NewMemoryStore(), false) mgmt := newManagement() d.PutStorageItem(mgmt.ID, []byte{PrefixContract}, state.StorageItem{0xFF}) require.Error(t, mgmt.InitializeCache(d)) @@ -98,7 +98,7 @@ func TestManagement_Initialize(t *testing.T) { func TestManagement_GetNEP17Contracts(t *testing.T) { mgmt := newManagement() mgmt.Policy = newPolicy() - d := dao.NewSimple(storage.NewMemoryStore(), false, false) + d := dao.NewSimple(storage.NewMemoryStore(), false) err := mgmt.Initialize(&interop.Context{DAO: d}) require.NoError(t, err) require.NoError(t, mgmt.Policy.Initialize(&interop.Context{DAO: d})) diff --git a/pkg/core/statesync/module_test.go b/pkg/core/statesync/module_test.go index 6894f0593..893dfe07e 100644 --- a/pkg/core/statesync/module_test.go +++ b/pkg/core/statesync/module_test.go @@ -54,7 +54,7 @@ func TestModule_PR2019_discussion_r689629704(t *testing.T) { syncPoint: 1000500, syncStage: headersSynced, syncInterval: 100500, - dao: dao.NewSimple(actualStorage, true, false), + dao: dao.NewSimple(actualStorage, true), mptpool: NewPool(), } stateSync.billet = mpt.NewBillet(sr, mpt.ModeLatest,