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

Monero: add more legacy verify functions #383

Merged
merged 16 commits into from
Nov 12, 2023
Merged

Conversation

Boog900
Copy link
Contributor

@Boog900 Boog900 commented Sep 28, 2023

Fixes #367

  • Adds support for verifying legacy ring signatures
  • Changes blocks major/ miner versions to u8 to match monero
  • Changes more of the varint code to be generic over the integer type
  • adds back support for multiple input aggregate MLSAG signatures
  • adds verifying for both types of MLSAG signatures
  • adds an UnreducedScalar type which has a method for recovering scalars which underwent an incorrect reduction

@kayabaNerve
Copy link
Member

no-std CI failure is legitimate, processor/coordinator/full-stack aren't (they're some Docker connectivity issue). I've re-run them.

@kayabaNerve
Copy link
Member

I appreciate 46f4370, yet if you want to make further RPC changes, I'd appreciate it on a distinct PR.

By the way, one of the biggest changes we can make for performance will be moving to get_blocks.bin.

Copy link
Member

@kayabaNerve kayabaNerve left a comment

Choose a reason for hiding this comment

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

This was a partial review I did weeks ago, sorry for the delay. I figured I may as well comment it now since I'm still rather occupied, sorry again. Mainly minor commentary :)

coins/monero/src/serialize.rs Show resolved Hide resolved
coins/monero/src/block.rs Show resolved Hide resolved
coins/monero/src/unreduced_scalar.rs Outdated Show resolved Hide resolved
coins/monero/src/unreduced_scalar.rs Outdated Show resolved Hide resolved
coins/monero/src/ringct/mlsag.rs Show resolved Hide resolved
coins/monero/src/ringct/borromean.rs Show resolved Hide resolved
coins/monero/src/ringct/borromean.rs Show resolved Hide resolved
coins/monero/src/ringct/borromean.rs Show resolved Hide resolved
coins/monero/src/ringct/borromean.rs Outdated Show resolved Hide resolved
Boog900 and others added 16 commits November 12, 2023 05:42
I have added this type for borromen sigs, the ee field can be a normal
scalar as in the verify function the ee
field is checked against a reduced scalar mean for it to verify as
correct ee must be reduced
this matches Monero

I have also changed a couple varint functions to accept the `VarInt`
trait
I still need to revert the commit removing support for >1 input MLSAG FULL

This adds a new rct type to separate Full and simple rct
also added `#[allow(clippy::needless_range_loop)]` around a loop as without a re-write satisfying clippy without it will make the function worse.
This function was causing me problems, every now and then a node would return a block with a different number than requested.
Monero calculates the POW hash and the block hash using *slightly* different blobs :/
@kayabaNerve
Copy link
Member

Will merge once CI passes 👍 Thanks for the effort :)

@kayabaNerve kayabaNerve merged commit 995734c into serai-dex:develop Nov 12, 2023
14 checks passed
@kayabaNerve kayabaNerve deleted the verify branch November 12, 2023 15:18
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.

MlsagAggregate improperly handled
2 participants