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

refactor: move mod-prm #44

Closed
wants to merge 25 commits into from
Closed

Conversation

ivokub
Copy link
Contributor

@ivokub ivokub commented Oct 31, 2023

WIP pull request. Do not merge!

@davidsemakula
Copy link
Contributor

@ivokub will this cover #38 ?
If so I will un-assign myself from that issue

@tmpfs tmpfs changed the base branch from main to develop November 7, 2023 09:02
@ivokub
Copy link
Contributor Author

ivokub commented Nov 7, 2023

@ivokub will this cover #38 ? If so I will un-assign myself from that issue

Indeed - I also changed the calls to Ring-Pedersen proof in this PR. Additionally, I defined RingPedersenParams in the new tss-core workspace which I'm using instead of carrying (N, s, t) everywhere. Imo makes more readable and allows better refactoring later.

@davidsemakula
Copy link
Contributor

Sounds good 👍
Minor pedantic suggestion, can we start switching to the modern Rust module style i.e. my_module.rs instead of my_module/mod.rs. We can start this with the new tss-core core crate and gradually update the others as we get to refactoring them (I can also do this in a separate PR after this one if it's a lot of monkey work 🙂)

@ivokub
Copy link
Contributor Author

ivokub commented Nov 7, 2023

Sounds good 👍 Minor pedantic suggestion, can we start switching to the modern Rust module style i.e. my_module.rs instead of my_module/mod.rs. We can start this with the new tss-core core crate and gradually update the others as we get to refactoring them (I can also do this in a separate PR after this one if it's a lot of monkey work 🙂)

I can do it in the PR. It is my first Rust project and I wasn't aware of the new style :). I agree that it creates less clutter.

@drewstone
Copy link
Contributor

How's it going on this? Any help needed @ivokub

@ivokub
Copy link
Contributor Author

ivokub commented Nov 22, 2023

How's it going on this? Any help needed @ivokub

Hi Drew, was at Devconnect and before that preparing for presentations. Now I'm back with backlog settling, so restarting effort 👍 .

@drewstone
Copy link
Contributor

@ivokub if you open PRs directly against the repo (and not in a fork), I can jump in a help fix up the CI. I think you should be in the github org just in case you weren't sure if it was possible.

@ivokub ivokub mentioned this pull request Jan 31, 2024
@ivokub
Copy link
Contributor Author

ivokub commented Jan 31, 2024

Closing in favor of #48 which is pushed to this repository for appending.

@ivokub ivokub closed this Jan 31, 2024
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.

3 participants