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

feat: precheck triple and presigs #802

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

ChaoticTempest
Copy link
Member

This adds previewing whether a node has a triple id or presignature id. This also has a limit so that the amount of previews should not be overwhelming and require large responses. The current limit is 128 which allows us to view our most recently completed owned triples and presignatures.

Note that with this change, all nodes need to be updated before being able to generate presignatures and consequently signatures again because there's a filtering that happens. Old nodes may not return anything or error out on this new /state function

DavidM-D
DavidM-D previously approved these changes Aug 6, 2024
Copy link
Contributor

@DavidM-D DavidM-D left a comment

Choose a reason for hiding this comment

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

This is a good PR, it solves an issue we're facing.

Later on we need to build a more robust way of nodes understanding what other nodes know, there are a bunch of papers on this, but it's a problem for another day.

Copy link
Collaborator

@volovyks volovyks left a comment

Choose a reason for hiding this comment

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

I have not seen all of it yet, but the idea is viable.

If we talk about design, this one solves our particular problem, but it is not solving other potential problems we can have in the future.

I was thinking about trying to create a protocol and then failing it fast. Each node can fail it by sending a message to all other nodes, or maybe only to the proposer. Proposer can recreate the protocol and start over.

ppca
ppca previously approved these changes Aug 7, 2024
Copy link
Contributor

@ppca ppca left a comment

Choose a reason for hiding this comment

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

LGTM except the 2 comments.

chain-signatures/node/src/web/mod.rs Show resolved Hide resolved
@ChaoticTempest ChaoticTempest dismissed stale reviews from ppca and DavidM-D via e967e83 August 7, 2024 09:03
@ChaoticTempest
Copy link
Member Author

I was thinking about trying to create a protocol and then failing it fast. Each node can fail it by sending a message to all other nodes, or maybe only to the proposer. Proposer can recreate the protocol and start over.

@volovyks yeah that would be message rejection, but the issue with that is you have to reach a threshold agreement on the failure of the protocol which requires waiting up to threshold participants messages coming back (maybe never if they decide not to reject even though it was invalid). This is quicker by just purely checking the state. Message rejection will be much harder to implement

@ChaoticTempest
Copy link
Member Author

for some reason integration tests are stalling on this PR

@volovyks
Copy link
Collaborator

volovyks commented Aug 7, 2024

We should not wait for T rejections, N-T+1 is enough.
We can also fail the protocol if any of the parties reject it. Yes, it is not ideal, but that is happening now anyway.

ailisp
ailisp previously approved these changes Aug 12, 2024
Copy link
Member

@ailisp ailisp left a comment

Choose a reason for hiding this comment

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

This preview ensures there are sufficient peers with presig ready to go and implementation looks solid

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.

5 participants