-
Notifications
You must be signed in to change notification settings - Fork 198
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
EN 7439 transaction routing fixes #2343
Conversation
iulianpascalau
commented
Oct 7, 2020
- removed the possibility to broadcast a transaction from any node in the network. Each transaction will get broadcast only if it will be routed to the sender's shard ID node.
- added unit and integration tests that prove that the transaction validation works as expected
… the network. Each transaction will get broadcast only if it will be routed to the sender's shard ID node.
integrationTests/testInitializer.go
Outdated
|
||
_, err := senderNode.SendTransaction(tx) | ||
if err != nil { | ||
log.Error("could not create transaction", "address", node.OwnAccount.Address, "error", err) |
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.
could not send?
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.
done
node/node.go
Outdated
@@ -824,7 +829,7 @@ func (n *Node) ValidateTransaction(tx *transaction.Transaction) error { | |||
core.MaxTxNonceDeltaAllowed, | |||
) | |||
if err != nil { | |||
return nil | |||
return fmt.Errorf("%w in Node.ValidateTransaction", err) |
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.
this will arrive in the API response. I don't think a regular user would be interested to know the method where the validation failed. Please add a log for the wrapped error and return it normally
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.
done. The error is now logged as warn as it is a programming error (non critical).
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.
System tests passed.