Repository navigation
Fix divergent behavior in integer backends - #49
Merged
Merged
Conversation
Previous impl went into infinite loop with negative numbers Signed-off-by: Denis Varlakov <cordial.egg6523@fastmail.com>
rug backend used to allow whitespaces in the integer encoding when provided to `Integer::from_str_radix`, even if the whitespace is in the middle of the number. This commit forbids any characters that are not ascii alphanum in the integer encoding. Signed-off-by: Denis Varlakov <cordial.egg6523@fastmail.com>
survived
force-pushed
the
fix-divergent-behavior
branch
2 times, most recently
from
October 16, 2026 15:36
5cb7dd6 to
6f0ad9f
Compare
survived
marked this pull request as ready for review
October 16, 2026 15:38
maurges
reviewed
Oct 16, 2026
Comment on lines
+79
to
+90
| // rug accepts strings that have whitespaces in them, integer is parsed while | ||
| // ignoring whitespaces, even in the middle of the number. This behavior is | ||
| // different from num-bigint backend, and it is strange. We manually restrict | ||
| // strings to be ascii alphanum | ||
| if !s | ||
| .chars() | ||
| .enumerate() | ||
| .skip_while(|&(i, c)| i == 0 && (c == '+' || c == '-')) | ||
| .all(|(_, c)| c.is_ascii_alphanumeric()) | ||
| { | ||
| return None; | ||
| } |
Contributor
There was a problem hiding this comment.
I'm not sure we actually care about this change. It's not like we want our backends to be exactly identical everywhere, just give the same numerical operations and serialize to the same format. If the deserialization of one is more permissive than another - that doesn't concern our usage
Contributor
Author
There was a problem hiding this comment.
It does concern our usage if we have signers with different backends running in the same cluster. In this case, malicious party may send a broadcast message which will cause one subset of parties to abort (cuz num-bigint doesn't accept whitespaces) and the other subset of signers will continue the protocol.
maurges
reviewed
Oct 16, 2026
survived
force-pushed
the
fix-divergent-behavior
branch
from
October 16, 2026 20:00
dcac262 to
c86601c
Compare
Signed-off-by: Denis Varlakov <cordial.egg6523@fastmail.com>
Signed-off-by: Denis Varlakov <cordial.egg6523@fastmail.com>
Signed-off-by: Denis Varlakov <cordial.egg6523@fastmail.com>
Signed-off-by: Denis Varlakov <cordial.egg6523@fastmail.com>
survived
marked this pull request as draft
October 19, 2026 11:52
Signed-off-by: Denis Varlakov <cordial.egg6523@fastmail.com>
Signed-off-by: Denis Varlakov <cordial.egg6523@fastmail.com>
Signed-off-by: Denis Varlakov <cordial.egg6523@fastmail.com>
Signed-off-by: Denis Varlakov <cordial.egg6523@fastmail.com>
Current implementation panics if n == 0, and hangs in infinite loop if n == 1 Signed-off-by: Denis Varlakov <cordial.egg6523@fastmail.com>
Signed-off-by: Denis Varlakov <cordial.egg6523@fastmail.com>
survived
force-pushed
the
fix-divergent-behavior
branch
from
October 19, 2026 12:21
c86601c to
90dc666
Compare
survived
marked this pull request as ready for review
October 19, 2026 12:25
Signed-off-by: Denis Varlakov <cordial.egg6523@fastmail.com>
Signed-off-by: Denis Varlakov <cordial.egg6523@fastmail.com>
maurges
approved these changes
Oct 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I used Claude to analyze if both integer backends exhibit the same behavior, and it found methods that diverge.
PR fixes divergent behavior. Review commit-by-commit. See commit message.
Note: in some places, I add panics into the code, but I believe it's justified (but ideally, we probably want to change methods to return(this was changed: methods return error if input is invalid)None)Also see the changelog.