-
Notifications
You must be signed in to change notification settings - Fork 88
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
feat(sequencer, charts)!: support uds for abci #1877
Conversation
3b54ffc
to
9a1a550
Compare
55b645e
to
e931f1c
Compare
e931f1c
to
6a0632a
Compare
6a0632a
to
3daee25
Compare
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.
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_addr
config with newabci_listener_url
config 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_ADDR
config variable, replaced withASTRIA_SEQUENCER_ABCI_LISTENER_URL
Related Issues
closes #1891