Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Additional logging for transactions #968

Merged
merged 3 commits into from
Apr 18, 2016
Merged

Additional logging for transactions #968

merged 3 commits into from
Apr 18, 2016

Conversation

tomusdrw
Copy link
Collaborator

No description provided.

@tomusdrw tomusdrw added the A0-pleasereview 🤓 Pull request needs code review. label Apr 17, 2016
to_value(&hash)
}
Err(e) => {
warn!("Error sending transaction: {:?}", e);
trace!(target: "tx", "Failed to import transaction {:?} (hash: {:?})", e, hash);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

json-rpc making traces/warnings with target: "tx"?

@NikVolf
Copy link
Contributor

NikVolf commented Apr 17, 2016

to me, warning should be spawned in the module where error occured
if say tx-queue is used somewhere else, there will be no warnings in the current implementation

@NikVolf NikVolf added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Apr 17, 2016
@tomusdrw
Copy link
Collaborator Author

Your right. Wanted to reply that we need it here, because we only want to log sender transactions issued via RPC, but having the logging in import_own_transaction method inside miner create is much better place to put it. Updated the PR. Thanks!

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Apr 17, 2016
@NikVolf NikVolf added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Apr 17, 2016
@arkpar arkpar merged commit 5b8ca74 into master Apr 18, 2016
@tomusdrw tomusdrw deleted the tx-tracing branch April 21, 2016 14:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants