-
Notifications
You must be signed in to change notification settings - Fork 2
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: make sign queue use less shared state #91
Conversation
…e/cleanup-testing-interface
…pc into phuong/chore/cleanup-testing-interface
…pc into phuong/chore/cleanup-testing-interface
…pc into phuong/chore/cleanup-testing-interface
…pc into phuong/chore/cleanup-testing-interface
…e/cleanup-testing-interface
…e/cleanup-testing-interface
…ig-net/sig-mpc into phuong/feat/protocol-as-task
…ig-net/sig-mpc into phuong/feat/protocol-as-task
…/protocol-as-task
…/protocol-as-task
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'm unsure if this is a narrow place, but it makes sense overall. We will need to implement such a design for messages.
I have a feeling that we will keep sign requests in Redis, but it's a good solution for now.
pub fn new() -> Self { | ||
Self::default() | ||
pub fn channel() -> (mpsc::Sender<SignRequest>, mpsc::Receiver<SignRequest>) { | ||
mpsc::channel(MAX_SIGN_REQUESTS) |
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.
What would the behavior be if we got more than 1024 requests? Will they be dropped or wait to be added?
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 believe they will be dropped
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.
yeah, they'll be dropped but we'll have a whole entire sweep about adding these configs to the contract when we have more demand for it
pub fn new() -> Self { | ||
Self::default() | ||
pub fn channel() -> (mpsc::Sender<SignRequest>, mpsc::Receiver<SignRequest>) { | ||
mpsc::channel(MAX_SIGN_REQUESTS) |
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 believe they will be dropped
The base branch was changed.
…/sign-queue-channel
e80f62e
to
5a70c08
Compare
Makes the sign queue less contentious with indexer. This allows signature manager to progress without indexer taking up the write lock for too long.
This makes it so that sign queue only gets created in the
SignatureManager
, where a channel(sign_tx, sign_rx)
is create between the indexer and the protocol loop. Thesign_tx
is used by the indexer to send messages over the channel tosign_rx
which is acquired by theSignatureManager
on creation and organizing of message requests