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.

Sign up for GitHub

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

Jump to bottom

VoteCast JS-Bindings #21

Merged
merged 10 commits into from Oct 26, 2022
Merged

VoteCast JS-Bindings #21

merged 10 commits into from Oct 26, 2022

Conversation

Mr-Leshiy
Copy link
Contributor

  • Update current wasm JS binding generation.
  • Add VoteCast certificate building (only public voting type)
  • Add fragment generation, transaction signing

@stevenj
Copy link
Collaborator

stevenj commented Oct 11, 2022

We will also need private voting type. Is this still a WIP?

@Mr-Leshiy
Copy link
Contributor Author

@stevenj yes it is in progress

@minikin
Copy link
Collaborator

minikin commented Oct 12, 2022

@Mr-Leshiy, FYI for WIP PRs we can use draft mode.

dkijania
dkijania reviewed Oct 19, 2022
View reviewed changes
Copy link
Contributor

@dkijania dkijania left a comment

Choose a reason for hiding this comment

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

From what i see we are missing e2e test on js layer.

I imagine critical one which utilized our sign transaction method and return signed fragment.. then we decode it and verify that it has public key signature and all input data is the same

@minikin
Copy link
Collaborator

minikin commented Oct 19, 2022

@cong-or @FelipeRosa @cameron1024 Can you review this one, please?

@minikin minikin added the review me PR is ready for review label Oct 20, 2022
@Mr-Leshiy
Copy link
Contributor Author

@dkijania Yes sure, it is a ongoing plan. We will start doing it after this PR will b merged

cameron1024
cameron1024 approved these changes Oct 25, 2022
View reviewed changes
Copy link
Contributor

@cameron1024 cameron1024 left a comment

Choose a reason for hiding this comment

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

LGTM

I don't have much experience with wasm_bindgen so I can't say if there's anything bad going on there, but the rest looks good 👍

src/chain-wallet-libs/bindings/wallet-js/src/certificates/vote_plan.rs Show resolved Hide resolved
src/chain-wallet-libs/bindings/wallet-js/src/lib.rs Outdated Show resolved Hide resolved
src/chain-wallet-libs/bindings/wallet-js/tests/certificates/vote_cast.rs Outdated Show resolved Hide resolved
src/chain-wallet-libs/bindings/wallet-js/tests/certificates/vote_cast.rs Outdated Show resolved Hide resolved
src/chain-wallet-libs/bindings/wallet-js/tests/certificates/vote_plan.rs Outdated Show resolved Hide resolved
saibatizoku
saibatizoku approved these changes Oct 25, 2022
View reviewed changes
Copy link
Contributor

@saibatizoku saibatizoku left a comment

Choose a reason for hiding this comment

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

Thank you!

@Mr-Leshiy Mr-Leshiy merged commit 34f1bcf into main Oct 26, 2022
@Mr-Leshiy Mr-Leshiy deleted the feature/npg-3426-js-binding branch October 26, 2022 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saibatizoku saibatizoku saibatizoku approved these changes

@cameron1024 cameron1024 cameron1024 approved these changes

@stevenj stevenj Awaiting requested review from stevenj

@cong-or cong-or Awaiting requested review from cong-or

@dkijania dkijania Awaiting requested review from dkijania

Assignees
No one assigned
Labels
review me PR is ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Mr-Leshiy @stevenj @minikin @saibatizoku @dkijania @cameron1024