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

Trace transaction replies in txSubmissionOutbound after transactions are sent #1863

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

intricate
Copy link
Contributor

I noticed that in #1688 I was mistakenly tracing TraceTxSubmissionOutboundSendMsgReplyTxs events before the transactions had been successfully sent.

@intricate intricate requested a review from mrBliss March 27, 2020 06:34
@intricate intricate self-assigned this Mar 27, 2020
@intricate intricate force-pushed the intricate/txsubout-trace-sendtxs branch from ee54f98 to a5cb1f8 Compare March 27, 2020 06:38
Copy link
Contributor

@mrBliss mrBliss left a comment

Choose a reason for hiding this comment

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

I see. Tracing afterwards makes sense, but I'm not sure it's worth making the code that much more complex (I mean adding the @m@ to SendMsgReplyTxs). What do you think @dcoutts?

-- Trace the transactions to be sent in the response.
traceWith tracer (TraceTxSubmissionOutboundSendMsgReplyTxs txs)
-- Trace the transactions to be sent in the response.
traceTxsSent = traceWith tracer (TraceTxSubmissionOutboundSendMsgReplyTxs txs)
Copy link
Contributor

Choose a reason for hiding this comment

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

This requires a comment (here or near L195) saying that tracing itself will be executed after sending the reply.

@intricate
Copy link
Contributor Author

I see. Tracing afterwards makes sense, but I'm not sure it's worth making the code that much more complex...

@mrBliss: Do you feel the same way about the approach in the first commit?

data ClientStTxs txid tx m a where
  SendMsgReplyTxs   :: [tx]
                    -> ([tx] -> m ())
                    -> ClientStIdle txid tx m a
                    -> ClientStTxs  txid tx m a

@intricate intricate force-pushed the intricate/txsubout-trace-sendtxs branch from a5cb1f8 to 8fad020 Compare March 27, 2020 15:48
@intricate intricate requested a review from coot March 27, 2020 16:53
@intricate intricate force-pushed the intricate/txsubout-trace-sendtxs branch from 8fad020 to fd05404 Compare April 1, 2020 15:33
@intricate intricate requested a review from dcoutts April 3, 2020 04:40
@intricate intricate force-pushed the intricate/txsubout-trace-sendtxs branch 2 times, most recently from 32719f7 to 05436f9 Compare April 7, 2020 15:18
@intricate intricate added networking tx-submission Issues related to tx-submission protocol labels Apr 7, 2020
@intricate intricate force-pushed the intricate/txsubout-trace-sendtxs branch from 05436f9 to f3e301e Compare April 9, 2020 15:43
@coot coot removed the networking label Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tx-submission Issues related to tx-submission protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants