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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
integrate blsttc/blstrs and remove mint::* #157
Merged
Merged
Conversation
This file contains 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
This commit integrates Ian Coleman's recent branch of blsttc that uses blstrs instead of bls12_381. blsttc now implements SecretKey and PublicKey as newtype wrappers around blstrs::Scalar and blstrs::G1Affine respectively. It also depends on changes added by Anselme and myself to blsttc, ringct and their dependencies. These changes primarily provide From impls for the blsttc newtype wrappers so that blsttc keys can be used cleanly with ringct functons that accept Into<Scalar> and Into<PublicKey>. Some notable sn_dbc changes: * we only use blsttc keys, no more Scalar/G1Affine. * Amount is now defined in ringct and re-exported in sn_dbc. * get rid of rand7, now rand8 everywhere * use ringct::Output::new() and TrueInput::new() * gets rid of BlsHelper, SecretKeyBlst, PublicKeyBlst
* remove PublicKeyBlstMappable * typedef KeyImage to blsttc::KeyImage * use blsttc::PublicKey in map keys instead of PublicKeyBlstMappable * remove unnecessary clone() on PublicKey's
Adds two TransactionBuilder methods: ::add_output_by_amount() and ::add_outputs_by_amount(). With these a caller does not need to construct an Output and the calling code is shorter and nicer. This commit also changes all the tests, example, bench to use the new methods.
* remove 'rand' dep on 0.7.x * change 'rand8' dep to 'rand' = 0.8.0 * change code to use 'rand', 'rng' not 'rand8', 'rng8'.
These dependencies no longer require subtle-ng and move us a big step
closer to being able to publish the sn_dbc crate.
The remaining issues are primarily the blstrs crate, which has a PR
pending upstream, and the xor_name crate, which also has a PR pending.
blsttc = {git = "https://github.com/dan-da/blsttc", branch = "sn_dbc_integration"}
blstrs = {git = "https://github.com/davidrusu/blstrs.git", branch = "bulletproofs-fixes"}
blst_ringct = {git = "https://github.com/maidsafe/blst-ringct"}
blst_bulletproofs = {git= "https://github.com/davidrusu/blst-bulletproofs.git",
branch = "bls12-381-curve"}
Now two flamegraphs are automatically generated when `cargo bench` is run
We realized that mint nodes no longer serve a useful purpose as the spentbook nodes now both verify and sign the transaction. By removing the mint nodes: 1. The client has less work to do. 2. Safe network elder nodes only have a single role and do less work. 3. library has less code, which is easier to learn/audit/maintain. 4. flexibility: spentbook can be any impl that generates a SpentProof. Changes: * bench TransactionVerifier::verify() instead of MintNode::reissue() * remove mint::* from mint-repl * make TransactionBuilder::build() return only a DbcBuilder * remove ReissueRequestBuilder * modify DbcBuilder to build using SpentProofs * add DbcBuilder::spent_proofs() * remove all 'mint', 'Mint' from code/comments. * remove mint sigs from Dbc * fuzz spent proofs in prop_dbc_verification() * remove obsolete/unused error variants * remove all lib code from mint.rs. (tests remain there for now) * add KeyManager error as string to Error:InvalidSpentProofSignature * remove mint sig verification from TransactionVerifier::verify()
davidrusu
reviewed
Mar 19, 2022
Merged
davidrusu
approved these changes
Mar 22, 2022
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.
Looks good 馃殺
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.
This PR includes and obsoletes #154. removes 827 LOC. 馃憤
Alternatively, if #154 were merged as-is, I could rebase this branch to include only the mint removal.
integrate blsttc/blstrs
highlights:
lowlights:
remove mint::*
feat: remove mint nodes and related data structures
We realized that mint nodes no longer serve a useful purpose
as the spentbook nodes now both verify and sign the transaction.
By removing the mint nodes: