From 3cd7cd009c6446358eb61a522c9747527541172f Mon Sep 17 00:00:00 2001 From: Varun Parthasarathy Date: Thu, 7 Jan 2021 17:59:29 -0800 Subject: [PATCH 01/14] add FindRelatedTransactions --- storage/modules/block_storage.go | 118 ++++++++++++++++++++++++++++++- 1 file changed, 115 insertions(+), 3 deletions(-) diff --git a/storage/modules/block_storage.go b/storage/modules/block_storage.go index 8875921a4..36531f244 100644 --- a/storage/modules/block_storage.go +++ b/storage/modules/block_storage.go @@ -54,6 +54,9 @@ const ( // blockSyncIdentifier is the identifier used to acquire // a database lock. blockSyncIdentifier = "blockSyncIdentifier" + + // backwardRelation is a relation from a child to a root transaction + backwardRelation = "backwardRelation" // prefix/root/child ) type blockTransaction struct { @@ -103,6 +106,17 @@ func getTransactionPrefix( ) } +func getBackwardRelationKey( + root *types.TransactionIdentifier, + child *types.TransactionIdentifier, +) []byte { + childHash := "" + if child != nil { + childHash = child.Hash + } + return []byte(fmt.Sprintf("%s/%s/%s", transactionNamespace, root.Hash, childHash)) +} + // BlockWorker is an interface that allows for work // to be done while a block is added/removed from storage // in the same database transaction as the change. @@ -798,7 +812,7 @@ func (b *BlockStorage) RemoveBlock( gctx, transaction, blockIdentifier, - txn.TransactionIdentifier, + txn, ) }) } @@ -956,6 +970,14 @@ func (b *BlockStorage) storeTransaction( blockIdentifier *types.BlockIdentifier, tx *types.Transaction, ) error { + backwardRelationKeys := b.getBackwardRelationKeys(tx) + for _, key := range backwardRelationKeys { + err := storeUniqueKey(ctx, transaction, key, []byte{}, true) + if err != nil { + return fmt.Errorf("error storing backward relation key %s: %w", key, err) + } + } + namespace, hashKey := getTransactionKey(blockIdentifier, tx.TransactionIdentifier) bt := &blockTransaction{ Transaction: tx, @@ -970,6 +992,19 @@ func (b *BlockStorage) storeTransaction( return storeUniqueKey(ctx, transaction, hashKey, encodedResult, true) } +func (b *BlockStorage) getBackwardRelationKeys(tx *types.Transaction) [][]byte { + var keys [][]byte + for _, relatedTx := range tx.RelatedTransactions { + if relatedTx.Direction == types.Forward { + continue + } + + keys = append(keys, getBackwardRelationKey(relatedTx.TransactionIdentifier, tx.TransactionIdentifier)) + } + + return keys +} + func (b *BlockStorage) pruneTransaction( ctx context.Context, transaction database.Transaction, @@ -993,10 +1028,17 @@ func (b *BlockStorage) removeTransaction( ctx context.Context, transaction database.Transaction, blockIdentifier *types.BlockIdentifier, - transactionIdentifier *types.TransactionIdentifier, + tx *types.Transaction, ) error { - _, hashKey := getTransactionKey(blockIdentifier, transactionIdentifier) + backwardRelationKeys := b.getBackwardRelationKeys(tx) + for _, key := range backwardRelationKeys { + err := transaction.Delete(ctx, key) + if err != nil { + return fmt.Errorf("error removing backward relation key %s: %w", key, err) + } + } + _, hashKey := getTransactionKey(blockIdentifier, tx.TransactionIdentifier) return transaction.Delete(ctx, hashKey) } @@ -1084,6 +1126,76 @@ func (b *BlockStorage) FindTransaction( return newestBlock, newestTransaction, nil } +func (b *BlockStorage) FindRelatedTransactions( + ctx context.Context, + transactionIdentifier *types.TransactionIdentifier, + db database.Transaction, +) (*types.BlockIdentifier, *types.Transaction, []*types.TransactionIdentifier, error) { + maxBlock, tx, err := b.FindTransaction(ctx, transactionIdentifier, db) + + children, err := b.getChildren(ctx, tx, db) + if err != nil { + return nil, nil, nil, err + } + + for _, child := range children { + childBlock, childTx, err := b.FindTransaction(ctx, child, db) + if err != nil { + return nil, nil, nil, err + } + + if maxBlock.Index < childBlock.Index { + maxBlock = childBlock + } + + if childTx == nil { + return nil, nil, nil, nil + } + + newChildren, err := b.getChildren(ctx, childTx, db) + if err != nil { + return nil, nil, nil, err + } + children = append(children, newChildren...) + } + + return maxBlock, tx, children, nil +} + +func (b *BlockStorage) getChildren( + ctx context.Context, + tx *types.Transaction, + db database.Transaction, +) ([]*types.TransactionIdentifier, error) { + var children []*types.TransactionIdentifier + for _, relatedTx := range tx.RelatedTransactions { + if relatedTx.Direction == types.Forward { + children = append(children, relatedTx.TransactionIdentifier) + } + } + + _, err := db.Scan( + ctx, + getBackwardRelationKey(tx.TransactionIdentifier, nil), + getBackwardRelationKey(tx.TransactionIdentifier, nil), + func(k []byte, v []byte) error { + ss := strings.Split(string(k), "/") + txHash := ss[len(ss)-1] + txId := &types.TransactionIdentifier{ Hash: txHash } + children = append(children, txId) + return nil + }, + false, + false, + ) + + if err != nil { + return nil, err + } + + return children, nil +} + func (b *BlockStorage) findBlockTransaction( ctx context.Context, blockIdentifier *types.BlockIdentifier, From b1775018ae594e5439557867201c7b5faba507c8 Mon Sep 17 00:00:00 2001 From: Varun Parthasarathy Date: Fri, 8 Jan 2021 15:05:00 -0800 Subject: [PATCH 02/14] fix nits --- storage/errors/errors.go | 4 +++ storage/modules/block_storage.go | 42 +++++++++++++++++++++++--------- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/storage/errors/errors.go b/storage/errors/errors.go index 25b3d5780..2cf2810c1 100644 --- a/storage/errors/errors.go +++ b/storage/errors/errors.go @@ -403,6 +403,8 @@ var ( ErrNothingToPrune = errors.New("nothing to prune") ErrPruningFailed = errors.New("pruning failed") ErrCannotPruneTransaction = errors.New("cannot prune transaction") + ErrCannotStoreBackwardRelation = errors.New("cannot store backward relation") + ErrCannotRemoveBackwardRelation = errors.New("cannot remove backward relation") BlockStorageErrs = []error{ ErrHeadBlockNotFound, @@ -438,6 +440,8 @@ var ( ErrNothingToPrune, ErrPruningFailed, ErrCannotPruneTransaction, + ErrCannotStoreBackwardRelation, + ErrCannotRemoveBackwardRelation, } ) diff --git a/storage/modules/block_storage.go b/storage/modules/block_storage.go index 36531f244..2d66d761a 100644 --- a/storage/modules/block_storage.go +++ b/storage/modules/block_storage.go @@ -106,6 +106,8 @@ func getTransactionPrefix( ) } +// getBackwardRelationKey returns a db key for a backwards relation. passing nil in for the +// child returns a prefix key. func getBackwardRelationKey( root *types.TransactionIdentifier, child *types.TransactionIdentifier, @@ -114,7 +116,7 @@ func getBackwardRelationKey( if child != nil { childHash = child.Hash } - return []byte(fmt.Sprintf("%s/%s/%s", transactionNamespace, root.Hash, childHash)) + return []byte(fmt.Sprintf("%s/%s/%s", backwardRelation, root.Hash, childHash)) } // BlockWorker is an interface that allows for work @@ -970,9 +972,9 @@ func (b *BlockStorage) storeTransaction( blockIdentifier *types.BlockIdentifier, tx *types.Transaction, ) error { - backwardRelationKeys := b.getBackwardRelationKeys(tx) + backwardRelationKeys := getBackwardRelationKeys(tx) for _, key := range backwardRelationKeys { - err := storeUniqueKey(ctx, transaction, key, []byte{}, true) + err := transaction.Set(ctx, key, []byte{}, true) if err != nil { return fmt.Errorf("error storing backward relation key %s: %w", key, err) } @@ -986,15 +988,20 @@ func (b *BlockStorage) storeTransaction( encodedResult, err := b.db.Encoder().Encode(namespace, bt) if err != nil { - return fmt.Errorf("%w: %v", storageErrs.ErrTransactionDataEncodeFailed, err) + return fmt.Errorf("%w: %v", storageErrs.ErrCannotStoreBackwardRelation, err) } return storeUniqueKey(ctx, transaction, hashKey, encodedResult, true) } -func (b *BlockStorage) getBackwardRelationKeys(tx *types.Transaction) [][]byte { +func getBackwardRelationKeys(tx *types.Transaction) [][]byte { var keys [][]byte for _, relatedTx := range tx.RelatedTransactions { + // skip if on another network + if relatedTx.NetworkIdentifier != nil { + continue + } + if relatedTx.Direction == types.Forward { continue } @@ -1030,11 +1037,11 @@ func (b *BlockStorage) removeTransaction( blockIdentifier *types.BlockIdentifier, tx *types.Transaction, ) error { - backwardRelationKeys := b.getBackwardRelationKeys(tx) + backwardRelationKeys := getBackwardRelationKeys(tx) for _, key := range backwardRelationKeys { err := transaction.Delete(ctx, key) if err != nil { - return fmt.Errorf("error removing backward relation key %s: %w", key, err) + return fmt.Errorf("%w: %v", storageErrs.ErrCannotRemoveBackwardRelation, err) } } @@ -1132,6 +1139,13 @@ func (b *BlockStorage) FindRelatedTransactions( db database.Transaction, ) (*types.BlockIdentifier, *types.Transaction, []*types.TransactionIdentifier, error) { maxBlock, tx, err := b.FindTransaction(ctx, transactionIdentifier, db) + if err != nil { + return nil, nil, nil, err + } + + if maxBlock == nil { + return nil, nil, nil, nil + } children, err := b.getChildren(ctx, tx, db) if err != nil { @@ -1144,14 +1158,14 @@ func (b *BlockStorage) FindRelatedTransactions( return nil, nil, nil, err } - if maxBlock.Index < childBlock.Index { - maxBlock = childBlock - } - if childTx == nil { return nil, nil, nil, nil } + if maxBlock.Index < childBlock.Index { + maxBlock = childBlock + } + newChildren, err := b.getChildren(ctx, childTx, db) if err != nil { return nil, nil, nil, err @@ -1162,6 +1176,7 @@ func (b *BlockStorage) FindRelatedTransactions( return maxBlock, tx, children, nil } +// TODO: add support for relations across multiple networks func (b *BlockStorage) getChildren( ctx context.Context, tx *types.Transaction, @@ -1169,6 +1184,11 @@ func (b *BlockStorage) getChildren( ) ([]*types.TransactionIdentifier, error) { var children []*types.TransactionIdentifier for _, relatedTx := range tx.RelatedTransactions { + // skip if on another network + if relatedTx.NetworkIdentifier != nil { + continue + } + if relatedTx.Direction == types.Forward { children = append(children, relatedTx.TransactionIdentifier) } From fe0df2bb61bc4e66be07b2ea1f4814288c09a010 Mon Sep 17 00:00:00 2001 From: Varun Parthasarathy Date: Tue, 12 Jan 2021 12:56:52 -0800 Subject: [PATCH 03/14] fix iteration logic --- storage/modules/block_storage.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/storage/modules/block_storage.go b/storage/modules/block_storage.go index 2d66d761a..c2be9f747 100644 --- a/storage/modules/block_storage.go +++ b/storage/modules/block_storage.go @@ -1006,7 +1006,10 @@ func getBackwardRelationKeys(tx *types.Transaction) [][]byte { continue } - keys = append(keys, getBackwardRelationKey(relatedTx.TransactionIdentifier, tx.TransactionIdentifier)) + keys = append( + keys, + getBackwardRelationKey(relatedTx.TransactionIdentifier, tx.TransactionIdentifier), + ) } return keys @@ -1152,7 +1155,14 @@ func (b *BlockStorage) FindRelatedTransactions( return nil, nil, nil, err } - for _, child := range children { + i := 0 + for { + if i >= len(children) { + break + } + child := children[i] + i += 1 + childBlock, childTx, err := b.FindTransaction(ctx, child, db) if err != nil { return nil, nil, nil, err @@ -1201,7 +1211,7 @@ func (b *BlockStorage) getChildren( func(k []byte, v []byte) error { ss := strings.Split(string(k), "/") txHash := ss[len(ss)-1] - txId := &types.TransactionIdentifier{ Hash: txHash } + txId := &types.TransactionIdentifier{Hash: txHash} children = append(children, txId) return nil }, From 0680a81b5efb59e5f192d2a9cfb91a30cc72e145 Mon Sep 17 00:00:00 2001 From: Varun Parthasarathy Date: Tue, 12 Jan 2021 13:43:04 -0800 Subject: [PATCH 04/14] fix lint --- storage/modules/block_storage.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/storage/modules/block_storage.go b/storage/modules/block_storage.go index c2be9f747..1e4629ebf 100644 --- a/storage/modules/block_storage.go +++ b/storage/modules/block_storage.go @@ -1161,7 +1161,7 @@ func (b *BlockStorage) FindRelatedTransactions( break } child := children[i] - i += 1 + i++ childBlock, childTx, err := b.FindTransaction(ctx, child, db) if err != nil { @@ -1211,8 +1211,8 @@ func (b *BlockStorage) getChildren( func(k []byte, v []byte) error { ss := strings.Split(string(k), "/") txHash := ss[len(ss)-1] - txId := &types.TransactionIdentifier{Hash: txHash} - children = append(children, txId) + txID := &types.TransactionIdentifier{Hash: txHash} + children = append(children, txID) return nil }, false, From efa7e9e59845d9afe82e5fb8fca5fab98a377f3a Mon Sep 17 00:00:00 2001 From: Varun Parthasarathy Date: Tue, 12 Jan 2021 18:12:01 -0800 Subject: [PATCH 05/14] add tests --- storage/modules/block_storage_test.go | 96 +++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/storage/modules/block_storage_test.go b/storage/modules/block_storage_test.go index 08c92ce6f..13ce0fd0e 100644 --- a/storage/modules/block_storage_test.go +++ b/storage/modules/block_storage_test.go @@ -128,6 +128,23 @@ func simpleTransactionFactory( } } +func addRelatedTransaction( + transaction *types.Transaction, + hash string, + direction types.Direction, +) *types.Transaction { + relatedTx := &types.RelatedTransaction{ + NetworkIdentifier: nil, + TransactionIdentifier: &types.TransactionIdentifier { + Hash: hash, + }, + Direction: direction, + } + + transaction.RelatedTransactions = append(transaction.RelatedTransactions, relatedTx) + return transaction +} + var ( genesisBlock = &types.Block{ BlockIdentifier: &types.BlockIdentifier{ @@ -902,3 +919,82 @@ func TestAtTip(t *testing.T) { assert.True(t, atTip) }) } + +func TestRelatedTransactions(t *testing.T) { + // setup + ctx := context.Background() + + newDir, err := utils.CreateTempDir() + assert.NoError(t, err) + defer utils.RemoveTempDir(newDir) + + database, err := newTestBadgerDatabase(ctx, newDir) + assert.NoError(t, err) + defer database.Close(ctx) + + storage := NewBlockStorage(database, blockWorkerConcurrency) + + t.Run("test forward and backward relations", func(t *testing.T) { + err = storage.SeeBlock(ctx, genesisBlock) + assert.NoError(t, err) + err = storage.AddBlock(ctx, genesisBlock) + assert.NoError(t, err) + + block1 := &types.Block{ + BlockIdentifier: &types.BlockIdentifier{ + Hash: "blah 1", + Index: 1, + }, + ParentBlockIdentifier: &types.BlockIdentifier{ + Hash: "blah 0", + Index: 0, + }, + Timestamp: 1, + Transactions: []*types.Transaction{ + addRelatedTransaction(simpleTransactionFactory("parentTx", "addr1", "100", &types.Currency{Symbol: "hello"}), "childTx", types.Forward), + simpleTransactionFactory("backwardRelative", "addr2", "100", &types.Currency{Symbol: "hello"}), + }, + } + err = storage.SeeBlock(ctx, block1) + assert.NoError(t, err) + err = storage.AddBlock(ctx, block1) + assert.NoError(t, err) + + block2 := &types.Block{ + BlockIdentifier: &types.BlockIdentifier{ + Hash: "blah 2", + Index: 2, + }, + ParentBlockIdentifier: &types.BlockIdentifier{ + Hash: "blah 1", + Index: 1, + }, + Timestamp: 1, + Transactions: []*types.Transaction{ + simpleTransactionFactory("childTx", "addr3", "100", &types.Currency{Symbol: "hello"}), + addRelatedTransaction(simpleTransactionFactory("backwardTx", "addr4", "100", &types.Currency{Symbol: "hello"}), "backwardRelative", types.Backward), + addRelatedTransaction(simpleTransactionFactory("badForward", "addr5", "100", &types.Currency{Symbol: "hello"}), "invalid", types.Forward), + }, + } + err = storage.SeeBlock(ctx, block2) + assert.NoError(t, err) + err = storage.AddBlock(ctx, block2) + assert.NoError(t, err) + + _, _, related, err := storage.FindRelatedTransactions(ctx, block1.Transactions[0].TransactionIdentifier, storage.db.ReadTransaction(ctx)) + assert.NoError(t, err) + assert.Equal(t, len(related), 1) + assert.Equal(t, related[0].Hash, block2.Transactions[0].TransactionIdentifier.Hash) + + _, _, related, err = storage.FindRelatedTransactions(ctx, block1.Transactions[1].TransactionIdentifier, storage.db.ReadTransaction(ctx)) + assert.NoError(t, err) + assert.Equal(t, len(related), 1) + assert.Equal(t, related[0].Hash, block2.Transactions[1].TransactionIdentifier.Hash) + + blockId, tx, related, err := storage.FindRelatedTransactions(ctx, block2.Transactions[2].TransactionIdentifier, storage.db.ReadTransaction(ctx)) + assert.NoError(t, err) + assert.Nil(t, blockId) + assert.Nil(t, tx) + assert.Empty(t, related) + }) +} From 521dfcc7a2a8a14bde6aa8a186db52ea4ca3e267 Mon Sep 17 00:00:00 2001 From: Varun Parthasarathy Date: Tue, 12 Jan 2021 18:15:28 -0800 Subject: [PATCH 06/14] lint --- storage/modules/block_storage_test.go | 67 +++++++++++++++++++++++---- 1 file changed, 58 insertions(+), 9 deletions(-) diff --git a/storage/modules/block_storage_test.go b/storage/modules/block_storage_test.go index 13ce0fd0e..cbade6c72 100644 --- a/storage/modules/block_storage_test.go +++ b/storage/modules/block_storage_test.go @@ -135,7 +135,7 @@ func addRelatedTransaction( ) *types.Transaction { relatedTx := &types.RelatedTransaction{ NetworkIdentifier: nil, - TransactionIdentifier: &types.TransactionIdentifier { + TransactionIdentifier: &types.TransactionIdentifier{ Hash: hash, }, Direction: direction, @@ -951,8 +951,22 @@ func TestRelatedTransactions(t *testing.T) { }, Timestamp: 1, Transactions: []*types.Transaction{ - addRelatedTransaction(simpleTransactionFactory("parentTx", "addr1", "100", &types.Currency{Symbol: "hello"}), "childTx", types.Forward), - simpleTransactionFactory("backwardRelative", "addr2", "100", &types.Currency{Symbol: "hello"}), + addRelatedTransaction( + simpleTransactionFactory( + "parentTx", + "addr1", + "100", + &types.Currency{Symbol: "hello"}, + ), + "childTx", + types.Forward, + ), + simpleTransactionFactory( + "backwardRelative", + "addr2", + "100", + &types.Currency{Symbol: "hello"}, + ), }, } err = storage.SeeBlock(ctx, block1) @@ -971,9 +985,32 @@ func TestRelatedTransactions(t *testing.T) { }, Timestamp: 1, Transactions: []*types.Transaction{ - simpleTransactionFactory("childTx", "addr3", "100", &types.Currency{Symbol: "hello"}), - addRelatedTransaction(simpleTransactionFactory("backwardTx", "addr4", "100", &types.Currency{Symbol: "hello"}), "backwardRelative", types.Backward), - addRelatedTransaction(simpleTransactionFactory("badForward", "addr5", "100", &types.Currency{Symbol: "hello"}), "invalid", types.Forward), + simpleTransactionFactory( + "childTx", + "addr3", + "100", + &types.Currency{Symbol: "hello"}, + ), + addRelatedTransaction( + simpleTransactionFactory( + "backwardTx", + "addr4", + "100", + &types.Currency{Symbol: "hello"}, + ), + "backwardRelative", + types.Backward, + ), + addRelatedTransaction( + simpleTransactionFactory( + "badForward", + "addr5", + "100", + &types.Currency{Symbol: "hello"}, + ), + "invalid", + types.Forward, + ), }, } err = storage.SeeBlock(ctx, block2) @@ -981,17 +1018,29 @@ func TestRelatedTransactions(t *testing.T) { err = storage.AddBlock(ctx, block2) assert.NoError(t, err) - _, _, related, err := storage.FindRelatedTransactions(ctx, block1.Transactions[0].TransactionIdentifier, storage.db.ReadTransaction(ctx)) + _, _, related, err := storage.FindRelatedTransactions( + ctx, + block1.Transactions[0].TransactionIdentifier, + storage.db.ReadTransaction(ctx), + ) assert.NoError(t, err) assert.Equal(t, len(related), 1) assert.Equal(t, related[0].Hash, block2.Transactions[0].TransactionIdentifier.Hash) - _, _, related, err = storage.FindRelatedTransactions(ctx, block1.Transactions[1].TransactionIdentifier, storage.db.ReadTransaction(ctx)) + _, _, related, err = storage.FindRelatedTransactions( + ctx, + block1.Transactions[1].TransactionIdentifier, + storage.db.ReadTransaction(ctx), + ) assert.NoError(t, err) assert.Equal(t, len(related), 1) assert.Equal(t, related[0].Hash, block2.Transactions[1].TransactionIdentifier.Hash) - blockId, tx, related, err := storage.FindRelatedTransactions(ctx, block2.Transactions[2].TransactionIdentifier, storage.db.ReadTransaction(ctx)) + blockId, tx, related, err := storage.FindRelatedTransactions( + ctx, + block2.Transactions[2].TransactionIdentifier, + storage.db.ReadTransaction(ctx), + ) assert.NoError(t, err) assert.Nil(t, blockId) assert.Nil(t, tx) From f9210fd473d2c1709e2c99d1b3001f962c925ae7 Mon Sep 17 00:00:00 2001 From: Varun Parthasarathy Date: Tue, 12 Jan 2021 18:19:12 -0800 Subject: [PATCH 07/14] lint 2 electric boogaloo --- storage/modules/block_storage_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/modules/block_storage_test.go b/storage/modules/block_storage_test.go index cbade6c72..13f539022 100644 --- a/storage/modules/block_storage_test.go +++ b/storage/modules/block_storage_test.go @@ -1036,13 +1036,13 @@ func TestRelatedTransactions(t *testing.T) { assert.Equal(t, len(related), 1) assert.Equal(t, related[0].Hash, block2.Transactions[1].TransactionIdentifier.Hash) - blockId, tx, related, err := storage.FindRelatedTransactions( + blockID, tx, related, err := storage.FindRelatedTransactions( ctx, block2.Transactions[2].TransactionIdentifier, storage.db.ReadTransaction(ctx), ) assert.NoError(t, err) - assert.Nil(t, blockId) + assert.Nil(t, blockID) assert.Nil(t, tx) assert.Empty(t, related) }) From 0fecf54a39443d9fb86652c79802bd295874d38c Mon Sep 17 00:00:00 2001 From: Varun Parthasarathy Date: Wed, 13 Jan 2021 11:01:53 -0800 Subject: [PATCH 08/14] tweaks --- storage/modules/block_storage.go | 36 ++++++++++++++++++--------- storage/modules/block_storage_test.go | 12 +++++++-- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/storage/modules/block_storage.go b/storage/modules/block_storage.go index 1e4629ebf..ce7adccd4 100644 --- a/storage/modules/block_storage.go +++ b/storage/modules/block_storage.go @@ -1140,50 +1140,62 @@ func (b *BlockStorage) FindRelatedTransactions( ctx context.Context, transactionIdentifier *types.TransactionIdentifier, db database.Transaction, -) (*types.BlockIdentifier, *types.Transaction, []*types.TransactionIdentifier, error) { - maxBlock, tx, err := b.FindTransaction(ctx, transactionIdentifier, db) +) (*types.BlockIdentifier, *types.Transaction, []*types.Transaction, error) { + rootBlock, tx, err := b.FindTransaction(ctx, transactionIdentifier, db) if err != nil { return nil, nil, nil, err } - if maxBlock == nil { + if rootBlock == nil { return nil, nil, nil, nil } - children, err := b.getChildren(ctx, tx, db) + childIds, err := b.getChildren(ctx, tx, db) if err != nil { return nil, nil, nil, err } + // create map of seen transactions to avoid duplicates + seen := make(map[string]bool) + children := []*types.Transaction{} + i := 0 for { - if i >= len(children) { + if i >= len(childIds) { break } - child := children[i] + childID := childIds[i] i++ - childBlock, childTx, err := b.FindTransaction(ctx, child, db) + // skip duplicates + if _, val := seen[childID.Hash]; !val { + seen[childID.Hash] = true + } else { + continue + } + + childBlock, childTx, err := b.FindTransaction(ctx, childID, db) if err != nil { return nil, nil, nil, err } - if childTx == nil { + if childBlock == nil { return nil, nil, nil, nil } - if maxBlock.Index < childBlock.Index { - maxBlock = childBlock + children = append(children, childTx) + if rootBlock.Index < childBlock.Index { + rootBlock = childBlock } newChildren, err := b.getChildren(ctx, childTx, db) if err != nil { return nil, nil, nil, err } - children = append(children, newChildren...) + childIds = append(childIds, newChildren...) } - return maxBlock, tx, children, nil + return rootBlock, tx, children, nil } // TODO: add support for relations across multiple networks diff --git a/storage/modules/block_storage_test.go b/storage/modules/block_storage_test.go index 13f539022..4fcc2b58f 100644 --- a/storage/modules/block_storage_test.go +++ b/storage/modules/block_storage_test.go @@ -1025,7 +1025,11 @@ func TestRelatedTransactions(t *testing.T) { ) assert.NoError(t, err) assert.Equal(t, len(related), 1) - assert.Equal(t, related[0].Hash, block2.Transactions[0].TransactionIdentifier.Hash) + assert.Equal( + t, + related[0].TransactionIdentifier.Hash, + block2.Transactions[0].TransactionIdentifier.Hash, + ) _, _, related, err = storage.FindRelatedTransactions( ctx, @@ -1034,7 +1038,11 @@ func TestRelatedTransactions(t *testing.T) { ) assert.NoError(t, err) assert.Equal(t, len(related), 1) - assert.Equal(t, related[0].Hash, block2.Transactions[1].TransactionIdentifier.Hash) + assert.Equal( + t, + related[0].TransactionIdentifier.Hash, + block2.Transactions[1].TransactionIdentifier.Hash, + ) blockID, tx, related, err := storage.FindRelatedTransactions( ctx, From 59d63ecdc6b4828776ba5d4d33e88d130ccb7787 Mon Sep 17 00:00:00 2001 From: Varun Parthasarathy Date: Wed, 13 Jan 2021 13:44:28 -0800 Subject: [PATCH 09/14] backward relation checking --- storage/modules/block_storage.go | 46 +++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/storage/modules/block_storage.go b/storage/modules/block_storage.go index ce7adccd4..c0c637fd3 100644 --- a/storage/modules/block_storage.go +++ b/storage/modules/block_storage.go @@ -972,12 +972,9 @@ func (b *BlockStorage) storeTransaction( blockIdentifier *types.BlockIdentifier, tx *types.Transaction, ) error { - backwardRelationKeys := getBackwardRelationKeys(tx) - for _, key := range backwardRelationKeys { - err := transaction.Set(ctx, key, []byte{}, true) - if err != nil { - return fmt.Errorf("error storing backward relation key %s: %w", key, err) - } + err := b.storeBackwardRelations(ctx, transaction, tx) + if err != nil { + return err } namespace, hashKey := getTransactionKey(blockIdentifier, tx.TransactionIdentifier) @@ -988,31 +985,50 @@ func (b *BlockStorage) storeTransaction( encodedResult, err := b.db.Encoder().Encode(namespace, bt) if err != nil { - return fmt.Errorf("%w: %v", storageErrs.ErrCannotStoreBackwardRelation, err) + return fmt.Errorf("%w: %v", storageErrs.ErrTransactionDataEncodeFailed, err) } return storeUniqueKey(ctx, transaction, hashKey, encodedResult, true) } -func getBackwardRelationKeys(tx *types.Transaction) [][]byte { - var keys [][]byte +func (b *BlockStorage) storeBackwardRelations( + ctx context.Context, + transaction database.Transaction, + tx *types.Transaction, +) error { + var backwardRelationKeys [][]byte for _, relatedTx := range tx.RelatedTransactions { // skip if on another network if relatedTx.NetworkIdentifier != nil { continue } - if relatedTx.Direction == types.Forward { continue } - keys = append( - keys, + // skip if related block not found + block, _, err := b.FindTransaction(ctx, relatedTx.TransactionIdentifier, transaction) + if err != nil { + return fmt.Errorf("%w: %v", storageErrs.ErrCannotStoreBackwardRelation, err) + } + if block == nil { + continue + } + + backwardRelationKeys = append( + backwardRelationKeys, getBackwardRelationKey(relatedTx.TransactionIdentifier, tx.TransactionIdentifier), ) } - return keys + for _, key := range backwardRelationKeys { + err := transaction.Set(ctx, key, []byte{}, true) + if err != nil { + return fmt.Errorf("%w: %v", storageErrs.ErrCannotStoreBackwardRelation, err) + } + } + + return nil } func (b *BlockStorage) pruneTransaction( @@ -1156,7 +1172,7 @@ func (b *BlockStorage) FindRelatedTransactions( } // create map of seen transactions to avoid duplicates - seen := make(map[string]bool) + seen := make(map[string]struct{}) children := []*types.Transaction{} i := 0 @@ -1169,7 +1185,7 @@ func (b *BlockStorage) FindRelatedTransactions( // skip duplicates if _, val := seen[childID.Hash]; !val { - seen[childID.Hash] = true + seen[childID.Hash] = struct{}{} } else { continue } From 23b279bb710dd77bbf5d34f60c73862cc9d825a8 Mon Sep 17 00:00:00 2001 From: Varun Parthasarathy Date: Wed, 13 Jan 2021 13:57:15 -0800 Subject: [PATCH 10/14] fix remove tx --- storage/modules/block_storage.go | 52 ++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/storage/modules/block_storage.go b/storage/modules/block_storage.go index c0c637fd3..d83baa3ae 100644 --- a/storage/modules/block_storage.go +++ b/storage/modules/block_storage.go @@ -972,7 +972,7 @@ func (b *BlockStorage) storeTransaction( blockIdentifier *types.BlockIdentifier, tx *types.Transaction, ) error { - err := b.storeBackwardRelations(ctx, transaction, tx) + err := storeBackwardRelations(ctx, transaction, tx) if err != nil { return err } @@ -991,10 +991,45 @@ func (b *BlockStorage) storeTransaction( return storeUniqueKey(ctx, transaction, hashKey, encodedResult, true) } -func (b *BlockStorage) storeBackwardRelations( +func storeBackwardRelations( ctx context.Context, transaction database.Transaction, tx *types.Transaction, +) error { + fn := func(ctx context.Context, transaction database.Transaction, key []byte) error { + err := transaction.Set(ctx, key, []byte{}, true) + if err != nil { + return fmt.Errorf("%w: %v", storageErrs.ErrCannotStoreBackwardRelation, err) + } + + return nil + } + + return modifyBackwardRelations(ctx, transaction, tx, fn) +} + +func removeBackwardRelations( + ctx context.Context, + transaction database.Transaction, + tx *types.Transaction, +) error { + fn := func(ctx context.Context, transaction database.Transaction, key []byte) error { + err := transaction.Delete(ctx, key) + if err != nil { + return fmt.Errorf("%w: %v", storageErrs.ErrCannotRemoveBackwardRelation, err) + } + + return nil + } + + return modifyBackwardRelations(ctx, transaction, tx, fn) +} + +func modifyBackwardRelations( + ctx context.Context, + transaction database.Transaction, + tx *types.Transaction, + fn func(ctx context.Context, transaction database.Transaction, key []byte) error, ) error { var backwardRelationKeys [][]byte for _, relatedTx := range tx.RelatedTransactions { @@ -1022,9 +1057,9 @@ func (b *BlockStorage) storeBackwardRelations( } for _, key := range backwardRelationKeys { - err := transaction.Set(ctx, key, []byte{}, true) + err := fn(ctx, transaction, key) if err != nil { - return fmt.Errorf("%w: %v", storageErrs.ErrCannotStoreBackwardRelation, err) + return err } } @@ -1056,12 +1091,9 @@ func (b *BlockStorage) removeTransaction( blockIdentifier *types.BlockIdentifier, tx *types.Transaction, ) error { - backwardRelationKeys := getBackwardRelationKeys(tx) - for _, key := range backwardRelationKeys { - err := transaction.Delete(ctx, key) - if err != nil { - return fmt.Errorf("%w: %v", storageErrs.ErrCannotRemoveBackwardRelation, err) - } + err := removeBackwardRelations(ctx, transaction, tx) + if err != nil { + return err } _, hashKey := getTransactionKey(blockIdentifier, tx.TransactionIdentifier) From 8cd50911fbdfcc2f51a68387e11e4e898c160c8c Mon Sep 17 00:00:00 2001 From: Varun Parthasarathy Date: Wed, 13 Jan 2021 14:00:45 -0800 Subject: [PATCH 11/14] fix build --- storage/modules/block_storage.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/storage/modules/block_storage.go b/storage/modules/block_storage.go index d83baa3ae..b06e96acc 100644 --- a/storage/modules/block_storage.go +++ b/storage/modules/block_storage.go @@ -972,7 +972,7 @@ func (b *BlockStorage) storeTransaction( blockIdentifier *types.BlockIdentifier, tx *types.Transaction, ) error { - err := storeBackwardRelations(ctx, transaction, tx) + err := b.storeBackwardRelations(ctx, transaction, tx) if err != nil { return err } @@ -991,7 +991,7 @@ func (b *BlockStorage) storeTransaction( return storeUniqueKey(ctx, transaction, hashKey, encodedResult, true) } -func storeBackwardRelations( +func (b *BlockStorage) storeBackwardRelations( ctx context.Context, transaction database.Transaction, tx *types.Transaction, @@ -1005,10 +1005,10 @@ func storeBackwardRelations( return nil } - return modifyBackwardRelations(ctx, transaction, tx, fn) + return b.modifyBackwardRelations(ctx, transaction, tx, fn) } -func removeBackwardRelations( +func (b *BlockStorage) removeBackwardRelations( ctx context.Context, transaction database.Transaction, tx *types.Transaction, @@ -1022,10 +1022,10 @@ func removeBackwardRelations( return nil } - return modifyBackwardRelations(ctx, transaction, tx, fn) + return b.modifyBackwardRelations(ctx, transaction, tx, fn) } -func modifyBackwardRelations( +func (b *BlockStorage) modifyBackwardRelations( ctx context.Context, transaction database.Transaction, tx *types.Transaction, @@ -1091,7 +1091,7 @@ func (b *BlockStorage) removeTransaction( blockIdentifier *types.BlockIdentifier, tx *types.Transaction, ) error { - err := removeBackwardRelations(ctx, transaction, tx) + err := b.removeBackwardRelations(ctx, transaction, tx) if err != nil { return err } From b81fbdf1b02f40a1dea093df80067801c91ef435 Mon Sep 17 00:00:00 2001 From: Varun Parthasarathy Date: Wed, 13 Jan 2021 17:22:47 -0800 Subject: [PATCH 12/14] add duplicate checks --- asserter/block.go | 22 +++++++++++++++ asserter/block_test.go | 62 ++++++++++++++++++++++++++++++++++++++++++ asserter/errors.go | 6 +++- 3 files changed, 89 insertions(+), 1 deletion(-) diff --git a/asserter/block.go b/asserter/block.go index fc951eae7..4a1f67b7c 100644 --- a/asserter/block.go +++ b/asserter/block.go @@ -370,6 +370,10 @@ func (a *Asserter) Transaction( // any of the related transactions contain invalid types, invalid network identifiers, // invalid transaction identifiers, or a direction not defined by the enum. func (a *Asserter) RelatedTransactions(relatedTransactions []*types.RelatedTransaction) error { + if dup := ContainsDuplicateRelatedTransaction(relatedTransactions); dup != nil { + return fmt.Errorf("%w: %v", ErrDuplicateRelatedTransaction, dup) + } + for i, relatedTransaction := range relatedTransactions { if relatedTransaction.NetworkIdentifier != nil { if err := NetworkIdentifier(relatedTransaction.NetworkIdentifier); err != nil { @@ -401,6 +405,24 @@ func (a *Asserter) RelatedTransactions(relatedTransactions []*types.RelatedTrans return nil } +// ContainsDuplicateCurrency returns nil if no duplicates are found in the array and +// returns the first duplicated item found otherwise. +func ContainsDuplicateRelatedTransaction( + items []*types.RelatedTransaction, +) *types.RelatedTransaction { + seen := map[string]struct{}{} + for _, item := range items { + key := types.Hash(item) + if _, ok := seen[key]; ok { + return item + } + + seen[key] = struct{}{} + } + + return nil +} + // Direction returns an error if the value passed is not types.Forward or types.Backward func (a *Asserter) Direction(direction types.Direction) error { if direction != types.Forward && diff --git a/asserter/block_test.go b/asserter/block_test.go index 933fbe402..b45cb5f02 100644 --- a/asserter/block_test.go +++ b/asserter/block_test.go @@ -727,6 +727,59 @@ func TestBlock(t *testing.T) { }, }, } + duplicateRelatedTransactions := &types.Transaction{ + TransactionIdentifier: &types.TransactionIdentifier{ + Hash: "blah", + }, + Operations: []*types.Operation{ + { + OperationIdentifier: &types.OperationIdentifier{ + Index: int64(0), + }, + Type: "PAYMENT", + Status: types.String("SUCCESS"), + Account: validAccount, + Amount: validAmount, + }, + { + OperationIdentifier: &types.OperationIdentifier{ + Index: int64(1), + }, + RelatedOperations: []*types.OperationIdentifier{ + { + Index: int64(0), + }, + }, + Type: "PAYMENT", + Status: types.String("SUCCESS"), + Account: validAccount, + Amount: validAmount, + }, + }, + RelatedTransactions: []*types.RelatedTransaction{ + { + NetworkIdentifier: &types.NetworkIdentifier{ + Blockchain: "hello", + Network: "world", + }, + TransactionIdentifier: &types.TransactionIdentifier{ + Hash: "blah", + }, + Direction: types.Forward, + }, + { + NetworkIdentifier: &types.NetworkIdentifier{ + Blockchain: "hello", + Network: "world", + }, + TransactionIdentifier: &types.TransactionIdentifier{ + Hash: "blah", + }, + Direction: types.Forward, + }, + }, + } + var tests = map[string]struct { block *types.Block genesisIndex int64 @@ -908,6 +961,15 @@ func TestBlock(t *testing.T) { }, err: ErrInvalidDirection, }, + "duplicate related transaction": { + block: &types.Block{ + BlockIdentifier: validBlockIdentifier, + ParentBlockIdentifier: validParentBlockIdentifier, + Timestamp: MinUnixEpoch + 1, + Transactions: []*types.Transaction{duplicateRelatedTransactions}, + }, + err: ErrDuplicateRelatedTransaction, + }, } for name, test := range tests { diff --git a/asserter/errors.go b/asserter/errors.go index 7b9271b7b..8f0b588b0 100644 --- a/asserter/errors.go +++ b/asserter/errors.go @@ -91,7 +91,10 @@ var ( ErrBlockIndexPrecedesParentBlockIndex = errors.New( "BlockIdentifier.Index <= ParentBlockIdentifier.Index", ) - ErrInvalidDirection = errors.New("invalid direction (must be 'forward' or 'backward')") + ErrInvalidDirection = errors.New( + "invalid direction (must be 'forward' or 'backward')", + ) + ErrDuplicateRelatedTransaction = errors.New("duplicate related transaction") BlockErrs = []error{ ErrAmountValueMissing, @@ -127,6 +130,7 @@ var ( ErrBlockHashEqualsParentBlockHash, ErrBlockIndexPrecedesParentBlockIndex, ErrInvalidDirection, + ErrDuplicateRelatedTransaction, } ) From d4431e84f41f4aaab2ffe02d10814fa7dd635ec7 Mon Sep 17 00:00:00 2001 From: Varun Parthasarathy Date: Thu, 14 Jan 2021 13:38:00 -0800 Subject: [PATCH 13/14] nits --- asserter/block.go | 2 +- storage/modules/block_storage.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/asserter/block.go b/asserter/block.go index 4a1f67b7c..975d9ebc1 100644 --- a/asserter/block.go +++ b/asserter/block.go @@ -405,7 +405,7 @@ func (a *Asserter) RelatedTransactions(relatedTransactions []*types.RelatedTrans return nil } -// ContainsDuplicateCurrency returns nil if no duplicates are found in the array and +// ContainsDuplicateRelatedTransaction returns nil if no duplicates are found in the array and // returns the first duplicated item found otherwise. func ContainsDuplicateRelatedTransaction( items []*types.RelatedTransaction, diff --git a/storage/modules/block_storage.go b/storage/modules/block_storage.go index b06e96acc..65419a557 100644 --- a/storage/modules/block_storage.go +++ b/storage/modules/block_storage.go @@ -1216,7 +1216,7 @@ func (b *BlockStorage) FindRelatedTransactions( i++ // skip duplicates - if _, val := seen[childID.Hash]; !val { + if _, ok := seen[childID.Hash]; !ok { seen[childID.Hash] = struct{}{} } else { continue From ce5cd47ab016b10afa5a46174b6b6767cb517e49 Mon Sep 17 00:00:00 2001 From: Varun Parthasarathy Date: Wed, 17 Feb 2021 12:52:01 -0800 Subject: [PATCH 14/14] address comments --- asserter/block.go | 6 +++--- storage/modules/block_storage.go | 25 ++++++++++++++----------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/asserter/block.go b/asserter/block.go index 975d9ebc1..2b8895522 100644 --- a/asserter/block.go +++ b/asserter/block.go @@ -370,7 +370,7 @@ func (a *Asserter) Transaction( // any of the related transactions contain invalid types, invalid network identifiers, // invalid transaction identifiers, or a direction not defined by the enum. func (a *Asserter) RelatedTransactions(relatedTransactions []*types.RelatedTransaction) error { - if dup := ContainsDuplicateRelatedTransaction(relatedTransactions); dup != nil { + if dup := DuplicateRelatedTransaction(relatedTransactions); dup != nil { return fmt.Errorf("%w: %v", ErrDuplicateRelatedTransaction, dup) } @@ -405,9 +405,9 @@ func (a *Asserter) RelatedTransactions(relatedTransactions []*types.RelatedTrans return nil } -// ContainsDuplicateRelatedTransaction returns nil if no duplicates are found in the array and +// DuplicateRelatedTransaction returns nil if no duplicates are found in the array and // returns the first duplicated item found otherwise. -func ContainsDuplicateRelatedTransaction( +func DuplicateRelatedTransaction( items []*types.RelatedTransaction, ) *types.RelatedTransaction { seen := map[string]struct{}{} diff --git a/storage/modules/block_storage.go b/storage/modules/block_storage.go index 65419a557..490ba0486 100644 --- a/storage/modules/block_storage.go +++ b/storage/modules/block_storage.go @@ -56,6 +56,8 @@ const ( blockSyncIdentifier = "blockSyncIdentifier" // backwardRelation is a relation from a child to a root transaction + // the root is the destination and the child is the transaction listing the root as a backward + // relation backwardRelation = "backwardRelation" // prefix/root/child ) @@ -109,14 +111,14 @@ func getTransactionPrefix( // getBackwardRelationKey returns a db key for a backwards relation. passing nil in for the // child returns a prefix key. func getBackwardRelationKey( - root *types.TransactionIdentifier, - child *types.TransactionIdentifier, + backwardTransaction *types.TransactionIdentifier, + tx *types.TransactionIdentifier, ) []byte { childHash := "" - if child != nil { - childHash = child.Hash + if tx != nil { + childHash = tx.Hash } - return []byte(fmt.Sprintf("%s/%s/%s", backwardRelation, root.Hash, childHash)) + return []byte(fmt.Sprintf("%s/%s/%s", backwardRelation, backwardTransaction.Hash, childHash)) } // BlockWorker is an interface that allows for work @@ -999,7 +1001,7 @@ func (b *BlockStorage) storeBackwardRelations( fn := func(ctx context.Context, transaction database.Transaction, key []byte) error { err := transaction.Set(ctx, key, []byte{}, true) if err != nil { - return fmt.Errorf("%w: %v", storageErrs.ErrCannotStoreBackwardRelation, err) + return fmt.Errorf("%v: %w", storageErrs.ErrCannotStoreBackwardRelation, err) } return nil @@ -1037,14 +1039,14 @@ func (b *BlockStorage) modifyBackwardRelations( if relatedTx.NetworkIdentifier != nil { continue } - if relatedTx.Direction == types.Forward { + if relatedTx.Direction != types.Backward { continue } // skip if related block not found block, _, err := b.FindTransaction(ctx, relatedTx.TransactionIdentifier, transaction) if err != nil { - return fmt.Errorf("%w: %v", storageErrs.ErrCannotStoreBackwardRelation, err) + return fmt.Errorf("%v: %w", storageErrs.ErrCannotStoreBackwardRelation, err) } if block == nil { continue @@ -1198,7 +1200,7 @@ func (b *BlockStorage) FindRelatedTransactions( return nil, nil, nil, nil } - childIds, err := b.getChildren(ctx, tx, db) + childIds, err := b.getForwardRelatedTransactions(ctx, tx, db) if err != nil { return nil, nil, nil, err } @@ -1236,7 +1238,7 @@ func (b *BlockStorage) FindRelatedTransactions( rootBlock = childBlock } - newChildren, err := b.getChildren(ctx, childTx, db) + newChildren, err := b.getForwardRelatedTransactions(ctx, childTx, db) if err != nil { return nil, nil, nil, err } @@ -1247,7 +1249,7 @@ func (b *BlockStorage) FindRelatedTransactions( } // TODO: add support for relations across multiple networks -func (b *BlockStorage) getChildren( +func (b *BlockStorage) getForwardRelatedTransactions( ctx context.Context, tx *types.Transaction, db database.Transaction, @@ -1264,6 +1266,7 @@ func (b *BlockStorage) getChildren( } } + // scan db for all transactions where tx appears as a backward relation _, err := db.Scan( ctx, getBackwardRelationKey(tx.TransactionIdentifier, nil),