From 2ad4c86712444edd03081a2deb5fc38b100467e5 Mon Sep 17 00:00:00 2001
From: Anna Shaleva <shaleva.ann@nspcc.ru>
Date: Wed, 15 May 2024 14:43:06 +0300
Subject: [PATCH] dao: move conflict record value length to a separate const

Conflicts-related code contains more and more these magic numbers, and
there's no good in it even if all the usages are commented. This
approach produces bugs like #3426.

No functional changes, just a refactoring.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
---
 pkg/core/dao/dao.go | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/pkg/core/dao/dao.go b/pkg/core/dao/dao.go
index 4adeec4e7..ba5713d93 100644
--- a/pkg/core/dao/dao.go
+++ b/pkg/core/dao/dao.go
@@ -35,6 +35,11 @@ var (
 	ErrInternalDBInconsistency = errors.New("internal DB inconsistency")
 )
 
+// conflictRecordValueLen is the length of value of transaction conflict record.
+// It consists of 1-byte [storage.ExecTransaction] prefix and 4-bytes block index
+// in the LE form.
+const conflictRecordValueLen = 1 + 4
+
 // Simple is memCached wrapper around DB, simple DAO implementation.
 type Simple struct {
 	Version Version
@@ -323,7 +328,7 @@ func (dao *Simple) GetTxExecResult(hash util.Uint256) (uint32, *transaction.Tran
 // decodeTxAndExecResult decodes transaction, its height and execution result from
 // the given executable bytes. It performs no executable prefix check.
 func decodeTxAndExecResult(buf []byte) (uint32, *transaction.Transaction, *state.AppExecResult, error) {
-	if len(buf) == 1+4 { // conflict record stub.
+	if len(buf) == conflictRecordValueLen { // conflict record stub.
 		return 0, nil, nil, storage.ErrKeyNotFound
 	}
 	r := io.NewBinReaderFromBuf(buf)
@@ -619,7 +624,7 @@ func (dao *Simple) GetTransaction(hash util.Uint256) (*transaction.Transaction,
 		// It may be a block.
 		return nil, 0, storage.ErrKeyNotFound
 	}
-	if len(b) == 1+4 { // storage.ExecTransaction + index
+	if len(b) == conflictRecordValueLen {
 		// It's a conflict record stub.
 		return nil, 0, storage.ErrKeyNotFound
 	}
@@ -699,7 +704,7 @@ func (dao *Simple) HasTransaction(hash util.Uint256, signers []transaction.Signe
 		return nil
 	}
 
-	if len(bytes) < 5 { // (storage.ExecTransaction + index) for conflict record
+	if len(bytes) < conflictRecordValueLen { // (storage.ExecTransaction + index) for conflict record
 		return nil
 	}
 	if bytes[0] != storage.ExecTransaction {
@@ -708,7 +713,7 @@ func (dao *Simple) HasTransaction(hash util.Uint256, signers []transaction.Signe
 		// we need to adjust Go behaviour post-factum. Ref. #3427 and 0x289c235dcdab8be7426d05f0fbb5e86c619f81481ea136493fa95deee5dbb7cc.
 		return nil
 	}
-	if len(bytes) != 5 {
+	if len(bytes) != conflictRecordValueLen {
 		return ErrAlreadyExists // fully-qualified transaction
 	}
 	if len(signers) == 0 {
@@ -864,7 +869,7 @@ func (dao *Simple) StoreAsTransaction(tx *transaction.Transaction, index uint32,
 	val := buf.Bytes()
 	dao.Store.Put(key, val)
 
-	val = val[:5] // storage.ExecTransaction (1 byte) + index (4 bytes)
+	val = val[:conflictRecordValueLen] // storage.ExecTransaction (1 byte) + index (4 bytes)
 	attrs := tx.GetAttributes(transaction.ConflictsT)
 	for _, attr := range attrs {
 		// Conflict record stub.