Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/runtime/transaction: Ensure consistent batch order indices #3248

Merged
merged 3 commits into from
Sep 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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