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

send txs out #31

Merged
merged 4 commits into from
Dec 7, 2023
Merged

send txs out #31

merged 4 commits into from
Dec 7, 2023

Conversation

dvush
Copy link
Contributor

@dvush dvush commented Dec 1, 2023

📝 Summary

Allows sending out collected txs.

--tx-receivers-allowed-sources is a list of tx sources to resend
--tx-receivers urls where to send txs (as a octet-stream POST request)

⛱ Motivation and Context

📚 References


✅ I have run these commands

  • make lint
  • make test
  • go mod tidy

@dvush dvush requested a review from metachris December 1, 2023 13:52
@@ -94,10 +111,40 @@ func (p *TxProcessor) Start() {
// start listening for transactions coming in through the channel
p.log.Info("Waiting for transactions...")
for txIn := range p.txC {
go p.sendTxToReceivers(txIn)
Copy link
Contributor

Choose a reason for hiding this comment

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

that place receives duplicates, it should probably be sent at https://github.com/flashbots/mempool-dumpster/blob/main/collector/tx_processor.go#L174

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this place in code is after a bunch of operations including writing to a file and request to a node

also duplicates count over all sources but we send only for allowed sources so this can lead to a big loss of txs

I think its better for receiver to do filtering then.

Copy link
Contributor

Choose a reason for hiding this comment

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

You are mistaken, the transactions are written to a file later, after deduplicating, inside the processTx function.

Your comment about loss of tx because filtering makes sense though. Would be good to add a comment about why this line of code is in this place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here where writing to a file https://github.com/flashbots/mempool-dumpster/blob/main/collector/tx_processor.go#L123 happens that I refer to
and its before deduplicating

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added comment 62915f4

@dvush dvush requested a review from metachris December 1, 2023 17:40
if err != nil {
return err
}
defer res.Body.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

To reuse the HTTP connection, you'll need to read the full body before closing: https://blog.cubieserver.de/2022/http-connection-reuse-in-go-clients/#:~:text=Client%20will%20only%20reuse%20connections,MaxConnsPerHost%20).

// ensure that we close the request so the httpClient can reuse the connection
io.Copy(io.Discard, res.Body)
res.Body.Close()

@dvush dvush requested a review from metachris December 4, 2023 11:36
collector/receiver.go Outdated Show resolved Hide resolved
@metachris metachris merged commit 7dd7061 into main Dec 7, 2023
2 checks passed
@metachris metachris deleted the resend-txs branch December 7, 2023 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants