Skip to content

Commit

Permalink
Merge pull request #3248 from oasisprotocol/kostko/fix/rt-txns-batcho…
Browse files Browse the repository at this point in the history
…rder

go/runtime/transaction: Ensure consistent batch order indices
  • Loading branch information
kostko authored Sep 8, 2020
2 parents 99f3bce + 3cbedf7 commit 581ecee
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 1 deletion.
1 change: 1 addition & 0 deletions .changelog/3248.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
go/runtime/transaction: Ensure consistent batch order indices
7 changes: 7 additions & 0 deletions go/oasis-node/cmd/debug/byzantine/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func newComputeBatchContext() *computeBatchContext {
func (cbc *computeBatchContext) receiveTransactions(ph *p2pHandle, timeout time.Duration) []*api.Tx {
var req p2pReqRes
txs := []*api.Tx{}
existing := make(map[hash.Hash]bool)

ReceiveTransactions:
for {
Expand All @@ -55,7 +56,13 @@ ReceiveTransactions:
if req.msg.Tx == nil {
continue
}
txHash := hash.NewFromBytes(req.msg.Tx.Data)
if existing[txHash] {
continue
}

txs = append(txs, req.msg.Tx)
existing[txHash] = true
case <-time.After(timeout):
break ReceiveTransactions
}
Expand Down
9 changes: 8 additions & 1 deletion go/runtime/transaction/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ const (

// MarshalBinary encodes an artifact kind into binary form.
func (ak artifactKind) MarshalBinary() (data []byte, err error) {
// kindInvalid should not be marshaleld.
// kindInvalid should not be marshaled.
if ak == kindInvalid {
return nil, errMalformedArtifactKind
}
Expand Down Expand Up @@ -258,6 +258,13 @@ func (t *Tree) GetInputBatch(ctx context.Context, maxBatchSize, maxBatchSizeByte

// Sort transactions to be in batch order.
sort.Stable(bo)

// Make sure that item orders are consistent.
for i, v := range bo.order {
if uint32(i) != v {
return nil, fmt.Errorf("transaction: inconsistent order: item %d has batch order %d", i, v)
}
}
bo.order = nil

return bo.batch, nil
Expand Down
21 changes: 21 additions & 0 deletions go/runtime/transaction/transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,3 +159,24 @@ func TestTransaction(t *testing.T) {
require.True(t, txnsByHash[checkTx.Hash()].Equal(&checkTx), "transaction should have the correct artifacts") // nolint: gosec
}
}

func TestTransactionInvalidBatchOrder(t *testing.T) {
ctx := context.Background()
store := mkvs.New(nil, nil)

var emptyRoot node.Root
emptyRoot.Empty()

tree := NewTree(store, emptyRoot)

tx := Transaction{
Input: []byte("this goes in"),
Output: []byte("and this comes out"),
BatchOrder: 1, // Invalid batch order as the first transaction should be at index zero.
}
err := tree.AddTransaction(ctx, tx, nil)
require.NoError(t, err, "AddTransaction")

_, err = tree.GetInputBatch(ctx, 0, 0)
require.Error(t, err, "GetInputBatch should fail with inconsistent order")
}
1 change: 1 addition & 0 deletions runtime/src/dispatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ impl Dispatcher {
) {
debug!(self.logger, "Received transaction batch request";
"state_root" => ?block.header.state_root,
"round" => block.header.round + 1,
"check_only" => check_only,
);

Expand Down

0 comments on commit 581ecee

Please sign in to comment.