-
Notifications
You must be signed in to change notification settings - Fork 95
feat(sequencer, charts)!: support uds for abci #1877
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
Conversation
3b54ffc to
9a1a550
Compare
55b645e to
e931f1c
Compare
e931f1c to
6a0632a
Compare
6a0632a to
3daee25
Compare
SuperFluffy
left a comment
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.
From a high level this looks fine, but I would like to see a slightly more elegant approach that is unit testable by providing an enum for the two supported server URL kinds.
SuperFluffy
left a comment
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.
I have added an enum AbciListenUrl with variants Tcp and Uds, together with a FromStr impl. I have also moved the parse to the config because that seemed to be the appropriate place for it (sequencer always runs by binding to a socket; lazy eval at a later point does not seem to make sense in this case).
Final change was from ASTRIA_SEQUENCER_ABCI_LISTENER_URL to ASTRIA_SEQUENCER_ABCI_LISTEN_URL because we are listening on that socket, not expecting a listener (CometBFT connects to the app, not the other way round).
Summary
Add support to the sequencer to support running the ABCI connection over UDS instead of TCP only, updates to chart to use the UDS connection by default.
Background
Local testing found up to a 25x speed up using UDS instead of TCP loopback for ABCI communication. We can continue to offer the option of using TCP connections, and support the faster connection within our charts by default.
Changes
listen_addrconfig with newabci_listener_urlconfig where protocol is used to inform sequencer whether to use tcp or unixTesting
Synced a full node with mainnet, and tested with ABCI replay.
Changelogs
Changelogs updated.
Breaking Changelist
ASTRIA_SEQUENCER_LISTEN_ADDRconfig variable, replaced withASTRIA_SEQUENCER_ABCI_LISTENER_URLRelated Issues
closes #1891