-
Notifications
You must be signed in to change notification settings - Fork 110
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
fix(dot/core): check transaction Validity.Propagate field to determine whether to propagate tx #1643
fix(dot/core): check transaction Validity.Propagate field to determine whether to propagate tx #1643
Changes from 16 commits
04b1aaf
1094eee
9417c83
39071d8
38850fe
f2b8582
aee53d3
49d5b71
bd77c71
5d84423
04a2750
7f32dae
9638b61
1f31c57
7dd754a
54ac571
e4bb319
43aeeef
1d74a8b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,19 +24,21 @@ import ( | |
|
||
// HandleTransactionMessage validates each transaction in the message and | ||
// adds valid transactions to the transaction queue of the BABE session | ||
func (s *Service) HandleTransactionMessage(msg *network.TransactionMessage) error { | ||
// returns boolean for transaction propagation, true - transactions should be propagoted | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed. |
||
func (s *Service) HandleTransactionMessage(msg *network.TransactionMessage) (bool, error) { | ||
logger.Debug("received TransactionMessage") | ||
|
||
// get transactions from message extrinsics | ||
txs := msg.Extrinsics | ||
|
||
var toPropagate []types.Extrinsic | ||
for _, tx := range txs { | ||
// validate each transaction | ||
externalExt := types.Extrinsic(append([]byte{byte(types.TxnExternal)}, tx...)) | ||
val, err := s.rt.ValidateTransaction(externalExt) | ||
if err != nil { | ||
logger.Error("failed to validate transaction", "err", err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd change this to debug since it's not an error There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point, updated. |
||
return err | ||
val.Propagate = false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated. |
||
continue | ||
} | ||
|
||
// create new valid transaction | ||
|
@@ -45,7 +47,14 @@ func (s *Service) HandleTransactionMessage(msg *network.TransactionMessage) erro | |
// push to the transaction queue of BABE session | ||
hash := s.transactionState.AddToPool(vtx) | ||
logger.Trace("Added transaction to queue", "hash", hash) | ||
|
||
// find tx(s) that should propagate | ||
if val.Propagate { | ||
toPropagate = append(toPropagate, tx) | ||
} | ||
} | ||
|
||
return nil | ||
msg.Extrinsics = toPropagate | ||
|
||
return len(msg.Extrinsics) > 0, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -157,8 +157,9 @@ func TestService_HandleTransactionMessage(t *testing.T) { | |
extBytes := createExtrinsics(t, s.rt, genHash, 0) | ||
|
||
msg := &network.TransactionMessage{Extrinsics: []types.Extrinsic{extBytes}} | ||
err = s.HandleTransactionMessage(msg) | ||
b, err := s.HandleTransactionMessage(msg) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you also add a test case for invalid extrinsics? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added. |
||
require.NoError(t, err) | ||
require.True(t, b) | ||
|
||
pending := s.transactionState.(*state.TransactionState).Pending() | ||
require.NotEqual(t, 0, len(pending)) | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to have a description that the bool return means there is extrinsic to propagate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point... Updated.