Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new algokit_algo25 Rust crate (and algokit_algo25_ffi) to support Algorand 25-word mnemonic ↔ seed conversions, along with generated Swift/Android bindings and updates to build/CI tooling to publish the new package.
Changes:
- Introduces
crates/algokit_algo25(mnemonic/seed conversion logic + wordlist) andcrates/algokit_algo25_ffi(UniFFI exports + error mapping). - Adds new Swift package
AlgoKitAlgo25and Android packagealgokit_algo25containing generated UniFFI bindings and build scaffolding. - Updates build tooling (
tools/build_pkgs) and CI workflow to includealgokit_algo25.
Reviewed changes
Copilot reviewed 22 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tools/build_pkgs/src/main.rs | Registers Algo25 as a buildable package and maps to algokit_algo25_ffi. |
| packages/swift/AlgoKitAlgo25/Tests/AlgoKitAlgo25Tests/AlgoKitAlgo25Tests.swift | Adds Swift test target skeleton for the new package. |
| packages/swift/AlgoKitAlgo25/Sources/AlgoKitAlgo25/AlgokitAlgo25.swift | Adds generated Swift UniFFI bindings for algo25. |
| packages/swift/AlgoKitAlgo25/Package.swift | Defines Swift package and binary target for the Algo25 xcframework. |
| packages/swift/AlgoKitAlgo25/.gitignore | Adds Swift package-specific ignores. |
| packages/android/algokit_algo25/src/main/kotlin/uniffi/algokit_algo25_ffi/algokit_algo25_ffi.kt | Adds generated Kotlin/JNA UniFFI bindings for algo25. |
| packages/android/algokit_algo25/settings.gradle.kts | Adds Gradle settings for the new Android module. |
| packages/android/algokit_algo25/gradlew.bat | Adds Gradle wrapper (Windows) for the new Android module. |
| packages/android/algokit_algo25/gradlew | Adds Gradle wrapper (POSIX) for the new Android module. |
| packages/android/algokit_algo25/gradle/wrapper/gradle-wrapper.properties | Configures Gradle wrapper distribution for the new module. |
| packages/android/algokit_algo25/gradle/wrapper/gradle-wrapper.jar | Adds Gradle wrapper JAR. |
| packages/android/algokit_algo25/gradle/libs.versions.toml | Adds version catalog for the new Android module. |
| packages/android/algokit_algo25/gradle.properties | Adds Gradle properties for the new Android module. |
| packages/android/algokit_algo25/build.gradle.kts | Adds Android library build configuration and deps (incl. JNA). |
| packages/android/algokit_algo25/README.md | Documents build and test commands for the Android package. |
| crates/algokit_algo25_ffi/uniffi.toml | Configures UniFFI Swift module name for algo25 FFI. |
| crates/algokit_algo25_ffi/src/lib.rs | Implements UniFFI-exported API surface and error conversions for algo25. |
| crates/algokit_algo25_ffi/Cargo.toml | Adds FFI crate manifest, crate types, and UniFFI feature wiring. |
| crates/algokit_algo25/src/lib.rs | Implements mnemonic/seed conversion logic and unit tests. |
| crates/algokit_algo25/src/english.rs | Adds English wordlist used for mnemonic encoding/decoding. |
| crates/algokit_algo25/Cargo.toml | Adds new crate manifest and sha2 dependency. |
| Cargo.toml | Registers new crates in the workspace members list. |
| Cargo.lock | Adds resolved dependencies for the new crates. |
| .github/workflows/ci_cd.yml | Includes algokit_algo25 in the CI list of FFI-producing crates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| } | ||
|
|
||
| android { | ||
| namespace = "com.example.algokit_algo25" |
There was a problem hiding this comment.
@joe-p is this supposed to be an example package name?
There was a problem hiding this comment.
What domain should we use?
There was a problem hiding this comment.
What domain should we use?
we are setup on mavencentral for io.github.algorandecosystem so I think our aar structure might be io.github.algorandecosystem:algokit-*
what about using io.github.algorandecosystem.algokit.algo25?
| material = { group = "com.google.android.material", name = "material", version.ref = "material" } | ||
|
|
||
| # Compose dependencies | ||
| androidx-compose-bom = { group = "androidx.compose", name = "compose-bom", version.ref = "compose-bom" } |
There was a problem hiding this comment.
@joe-p compose is for Android UI libraries...is this generated from Rust FFI? I think maybe we can remove them if it's not needed. Same with material and appcompat, expresso is UI testing
There was a problem hiding this comment.
This is probably leftover from when we created that first example app, so it's probably safe to remove
There was a problem hiding this comment.
Removed in 782b1ae. Let me know if there's anything I missed (or shouldn't have removed)
PhearZero
left a comment
There was a problem hiding this comment.
Looks good overall, checksum seems valid and the explicit check for 25 words was a nice touch!
| } | ||
|
|
||
| /// Takes a master derivation key and returns the corresponding mnemonic. | ||
| pub fn master_derivation_key_to_mnemonic(mdk: &[u8]) -> Result<String, MnemonicError> { |
There was a problem hiding this comment.
Is it just for consistent naming or is this something to be added later? I remember the sdk has a derivation function similar to kmd, I'm guessing this is a stub
There was a problem hiding this comment.
These functions are here to align with TS/Py which in turn are aligned with algosdk: https://github.com/algorand/js-algorand-sdk//blob/da8f2e6cd616e9430f8bb8d56bb9d25dffe1b98a/src/mnemonic/mnemonic.ts#L164-L181
I assume they exist because the master key and an ed25519 key are different things technically even though they use the same algorithm
There was a problem hiding this comment.
Ahh that's interesting. I wouldn't have expected that, yea it must be a naming decision. I'm guessing KMD does the same
| } | ||
|
|
||
| /// Takes a mnemonic and returns the corresponding master derivation key. | ||
| pub fn mnemonic_to_master_derivation_key( |
There was a problem hiding this comment.
is master_derivation_key only used for hd too? and not algo25?
i needed a way to get Algo25 public key from mnemonic and thought this was it, but AI was saying it's not the right function, but for BIP39 seed
There was a problem hiding this comment.
See this comment: #335 (comment)
I'll be adding HD support in the next PR
There was a problem hiding this comment.
See this comment: #335 (comment)
I'll be adding HD support in the next PR
oh ok
one question then...i thought this crate was only algo25 based off name (e.g. crates/algokit_algo25/src/lib.rs...)...will you rename if it includes HD functions too (e.g. crates/algokit_account/src/lib.rs...)? or will hd be it's own crate (e.g. crates/algokit_hd/src/lib.rs...)?
There was a problem hiding this comment.
Good question... maybe we rename this package to algokit_recovery and it'll have functions for both HD and algo25 mnemonics?
There was a problem hiding this comment.
Good question... maybe we rename this package to algokit_recovery and it'll have functions for both HD and algo25 mnemonics?
i think algokit_account might be better since if you add the random function...it'll have create account functionality in a way too. Or algokit_wallet / algokit_address? Solana has Seed Vault, maybe algokit_seed could work too?
then algokit_accountdetail or algokit_accountinfo can have balance, rekey flag, ASAs...or anything beyond account keys/addresses? what do you think? or maybe ASA becomes it's own crate...i'm open-minded
Adds the algo25 package which is used to go from mneomic to seed (and vice versa)