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

Wallet library #492

Merged
merged 18 commits into from
Dec 15, 2023
Merged

Wallet library #492

merged 18 commits into from
Dec 15, 2023

Conversation

orhoj
Copy link
Contributor

@orhoj orhoj commented Dec 12, 2023

Purpose

Migrate wallet functionality from https://github.com/Concordium/concordium-node-sdk-js/tree/main/packages/rust-bindings to concordium-base. This is done as the Java SDK and the upcoming Swift SDK will also depend on the same functionality.

All the functions are expected to be accessed through FFI from each native platform.

Changes

  • Migrated functionality from rust-bindings to concordium-base.
  • Refactored some of the functions.
  • Added functions that take the key material directly instead of the seed phrase. This was feedback from Frontier Wallet as they did not want to send the seed phrase to an outside library. It is also a sound change that we will probably use in the unified wallet as well.
  • Added some unit testing. I have tried to copy the tests from the node JS SDK to validate that functionality hasn't changed.
  • Added a FromStr implementation for Net in the key_derivation library.

Checklist

  • My code follows the style of this project.
  • The code compiles without warnings.
  • I have performed a self-review of the changes.
  • I have documented my code, in particular the intent of the
    hard-to-understand areas.
  • (If necessary) I have updated the CHANGELOG.

abizjak
abizjak reviewed Dec 13, 2023
View reviewed changes
rust-src/key_derivation/CHANGELOG.md Show resolved Hide resolved
rust-src/key_derivation/src/lib.rs Outdated Show resolved Hide resolved
rust-src/key_derivation/src/lib.rs Outdated Show resolved Hide resolved
rust-src/wallet_library/Cargo.toml Outdated Show resolved Hide resolved
rust-src/wallet_library/src/test_helpers.rs Outdated Show resolved Hide resolved
rust-src/wallet_library/src/wallet.rs Outdated Show resolved Hide resolved
rust-src/wallet_library/src/wallet.rs Outdated Show resolved Hide resolved
rust-src/wallet_library/src/identity.rs Outdated Show resolved Hide resolved
rust-src/wallet_library/src/wallet.rs Outdated Show resolved Hide resolved
rust-src/wallet_library/src/wallet.rs Show resolved Hide resolved
@orhoj
Refactoring according to review
9205a03 
key_derivation gets a minor bump
Better FromStr impl for Net
Add comments for structs
Some struct refactoring to align approach
Aligned method naming
Removed duplicated provider index from credential input
@orhoj orhoj merged commit 7bd133e into main Dec 15, 2023
40 checks passed
@orhoj orhoj deleted the wallet-library branch December 15, 2023 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abizjak abizjak abizjak approved these changes

@shjortConcordium shjortConcordium Awaiting requested review from shjortConcordium

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@orhoj @abizjak