Feat: add algorand chain and AVM chain type support#124
Feat: add algorand chain and AVM chain type support#124emg110 wants to merge 10 commits intoopen-wallet-standard:mainfrom
Conversation
|
@emg110 is attempting to deploy a commit to the MoonPay Team on Vercel. A member of the Team first needs to authorize it. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
njdawn
left a comment
There was a problem hiding this comment.
some findings from quick review - could you validate the below?
High: AvmSigner::default_derivation_path(index) returns m/44'/283'/{index}'/0/0, but the PR body itself describes the intended BIP-44 form as m/44'/283'/account'/0/index. Every other signer also uses index as the address index. As written, callers requesting index 1 will derive a different account, not the next address.
Medium: sign_transaction() signs the provided bytes and encode_signed_transaction() returns only the raw signature bytes, leaving msgpack signed-transaction assembly to the caller. That is a much looser contract than other chains and is easy to misuse from generic OWS transaction flows.
Medium: The Ed25519-BIP32 signing path acknowledges that it does not have the full extended key material and substitutes a derived nonce source. That needs stronger cross-validation against official Algorand tooling before it should be treated as production-grade.
|
Thank you very much @njdawn 🙏 |
|
Thank you @njdawn for very constructive comments 🙏 Finding 1 (High):
|
| Chain | Path Template | index Represents |
Source |
|---|---|---|---|
| AVM | m/44'/283'/{index}'/0/0 |
account | ows/crates/ows-signer/src/chains/avm.rs:154 |
| Solana | m/44'/501'/{index}'/0' |
account | ows/crates/ows-signer/src/chains/solana.rs:145 |
| Sui | m/44'/784'/{index}'/0'/0' |
account | ows/crates/ows-signer/src/chains/sui.rs:161 |
| TON | m/44'/607'/{index}' |
account | ows/crates/ows-signer/src/chains/ton.rs:201 |
// solana.rs:145
format!("m/44'/501'/{}'/0'", index)
// sui.rs:161
format!("m/44'/784'/{}'/0'/0'", index)
// ton.rs:201
format!("m/44'/607'/{}'", index)Algorand test coverage identically matches the output of test coverage on referenced repositories from Algorand Foundation on derivation results.
Finding 2 (Medium): sign_transaction / encode_signed_transaction Contract
Your Comment
sign_transaction()signs the provided bytes andencode_signed_transaction()returns only the raw signature bytes, leaving msgpack signed-transaction assembly to the caller. That is a much looser contract than other chains and is easy to misuse from generic OWS transaction flows.
Valid point. Both issues addressed and fixed in Commit: 3eb707d
2a. sign_transaction / sign_message — Domain Separation Prefixes
The initial implementation delegated domain separation to the caller — they had to prepend "TX" or "MX" themselves before calling sign_transaction or sign_message. This mirrored the AF's lower-level signAlgoTransaction(prefixEncodedTx) convention but, as the reviewer notes, is easy to misuse in a generic multi-chain signer where callers should not need to know Algorand-specific prefixing rules.
Resolution: sign_transaction now prepends "TX" and sign_message now prepends "MX" internally. The caller passes raw msgpack bytes (for transactions) or raw message bytes; the signer handles Algorand domain separation:
| Method | Prefix | Algorand Convention |
|---|---|---|
sign_transaction |
"TX" |
Transaction signing — all Algorand wallets prepend this |
sign_message |
"MX" |
Arbitrary message signing (ARC-60 convention) |
Commit: 3eb707d
2b. encode_signed_transaction — Now Produces Broadcast-Ready Msgpack
The initial implementation returned only the raw 64-byte signature, requiring the application to assemble the Algorand signed transaction msgpack structure itself. It was indeed looser contract than other chains. OWS is the signing entity — what it signs should be ready to send to algod.
Resolution: encode_signed_transaction now produces a complete, canonical msgpack-encoded signed transaction ready for algod's POST /v2/transactions endpoint:
msgpack({ "sig": <64-byte Ed25519 signature>, "txn": <transaction object> })
The msgpack envelope is hand-encoded (no external dependency needed) since the structure is fixed:
fn encode_signed_transaction(&self, tx_bytes: &[u8], signature: &SignOutput) -> Result<Vec<u8>, SignerError> {
// Hand-encoded canonical msgpack:
// 0x82 - fixmap with 2 entries
// 0xa3 "sig" - fixstr(3) "sig"
// 0xc4 0x40 <64 bytes> - bin8, length 64, signature
// 0xa3 "txn" - fixstr(3) "txn"
// <tx_bytes> - raw msgpack transaction object
// Keys sorted alphabetically ("sig" < "txn") per Algorand canonical encoding.
...
}The tx_bytes parameter is the msgpack-encoded unsigned transaction object (the same bytes passed to sign_transaction). The output can be sent directly to algod.
Finding 3 (Medium): Substituted Nonce Source in Ed25519-BIP32 Signing
Your Comment
The Ed25519-BIP32 signing path acknowledges that it does not have the full extended key material and substitutes a derived nonce source. That needs stronger cross-validation against official Algorand tooling before it should be treated as production-grade.
Valid point. Addressed and fixed in Commit: 3eb707d
What the AF's Implementation Does
We cross-validated against:
@algorandfoundation/xhd-wallet-apiv2.0.0-canary.1 — the AF's reference HD wallet library- Rocca — the AF's production mobile wallet (depends on the above)
The xHD-Wallet-API-ts rawSign() function (x.hd.wallet.api.crypto.ts:137-159) uses the real kR from the full 96-byte extended key. The AF's rawSign receives the full extended key from deriveKey() — 96 bytes containing kL, kR, and chainCode. It uses kR directly as the nonce source (SHA-512(kR || data)).
The applied Fix
The ChainSigner trait accepts &[u8] — it does not enforce 32 bytes. The fix is straightforward:
HdDeriver::derive_bip32_ed25519— return the full 96 bytes[kL(32) || kR(32) || chainCode(32)]instead of onlykLAvmSigner::sign_extended— extractkL(bytes 0..32) andkR(bytes 32..64) from the 96-byte input, usekRas the nonce source exactly as the AF'srawSign()doesAvmSigner::public_key_from_scalarandAvmSigner::derive_address— extractkLfrom the first 32 bytes of the extended key
This makes our signing output byte-for-byte identical to the AF's implementation for the same key and message.
Summary Table
| # | Severity | Finding | Response | Action Required |
|---|---|---|---|---|
| 1 | High | default_derivation_path uses account not address index |
Clarified — matches AF's Rocca wallet and xHD-Wallet-API convention exactly. | None. Clarified in this response. |
| 2a | Medium | sign_transaction doesn't prepend "TX" |
Fixed — Indeed Internal prefixing is safer. Adding "TX" to sign_transaction and "MX" to sign_message. |
Done. |
| 2b | Medium | encode_signed_transaction returns raw sig |
Fixed — OWS is the signer; output should be broadcast-ready. Now produces canonical msgpack signed transaction for algod. | Done. |
| 3 | Medium | Substituted nonce source | Fixed — Fixed to use real kR matching AF's rawSign() exactly. |
Done. |
Algorand Foundation sources referenced:
- @algorandfoundation/Rocca — AF's mobile wallet for identity and credentials which uses HD wallets
@algorandfoundation/xhd-wallet-apiv2.0.0-canary.1 — AF's reference HD wallet cryptography library@algorandfoundation/keystorev1.0.0-canary.12 — AF's keystore abstraction@algorandfoundation/react-native-keystorev1.0.0-canary.6 — AF's native keystore implementation
njdawn
left a comment
There was a problem hiding this comment.
please rebase on main to fix merge conflicts!
|
Surely and gladly! Thank you @njdawn |
95ddd89 to
3a797c1
Compare
3a797c1 to
7ee119b
Compare
|
Dear @njdawn , all is ready now
|
feat: add Algorand chain and AVM chain type support
Closes #101
This PR introduces AVM (Algorand Virtual Machine) as a supported chain type in the Open Wallet Standard, with Algorand as the first chain. The
ChainType::Avmfollows the same pattern asChainType::Evm— a single chain type that can support multiple chains sharing the same VM (e.g., Algorand, Voi). The implementation requires no new CLI commands — existing commands work with--chain algorandor--chain avm.Key Capabilities
ows sign message/tx --chain algorand(or--chain avm)ows wallet createandows mnemonic deriveTechnical Approach
HD Derivation: Implements BIP32-Ed25519 (Khovratovich 2017) with Peikert's amendment (g=9) as specified in ARC-52. Unlike SLIP-10 Ed25519 used by Solana/Sui, BIP32-Ed25519 supports both hardened and non-hardened child derivation, which is required for Algorand's BIP-44 path structure (
m/44'/283'/account'/0/index).Signing: Uses Ed25519 through
ed25519-dalek's hazmat API with the full 96-byte BIP32-Ed25519 extended key (kL || kR || chainCode). The realkRis used as the RFC 8032 nonce source, matching the Algorand Foundation'srawSign()implementation exactly.sign_transactionprepends the"TX"domain separator internally;sign_messageprepends"MX"(ARC-60).encode_signed_transactionproduces canonical msgpack ready for algod'sPOST /v2/transactions.Address format: Base32-encoded public key with 4-byte SHA-512/256 checksum (58 characters, no padding).
Chain Specifications
Avm(likeEvm— supports multiple AVM chains)algorand:wGHE2Pwdvd7S12BL5FaOP20EGYesN73kFiles Changed
12 files across 4 crates + Node bindings (~900 lines):
ChainType::Avmregistration, CAIP-2 mapping (algorand:namespace), RPC endpoint,"avm"alias inparse_chain/FromStrAvmSigner(chains/avm.rs),Curve::Ed25519Bip32Ed25519Bip32curve handling, broadcast stubalgorand:chain ID checkTesting
31 AVM/Algorand-specific tests covering root key generation, child derivation (6 address paths + 5 identity paths), extended private keys, address encoding, signing with real kR nonce, TX/MX domain separation, broadcast-ready msgpack encoding, and verification
All 500 workspace tests pass (247 in ows-signer, 135 in ows-lib, 67 in ows-core, 45 in ows-pay, 6 in ows-cli)
All 16 Node binding tests pass (updated for 9 chains)
Clippy clean with
-D warningsCross-verified against the Algorand Foundation's TypeScript reference implementation (algorandfoundation/xHD-Wallet-API-ts, 122/122 tests pass) and AF's production wallet (Rocca) — all derived public keys match exactly
E2E tested on Algorand testnet — OWS-derived keys successfully signed and submitted transactions to algod (confirmed on-chain)
Test vectors use the same mnemonic and BIP-44 paths as the reference implementation
cargo test --workspacepasses (500 tests, 0 failures)cargo clippy --workspace -- -D warningsis cleannpm testpasses (16/16 — Node binding tests updated for AVM)Tested manually with
owsCLI (ows mnemonic derive --chain algorand,--chain avm, and all-chains)Notes
New Dependencies
curve25519-daleked25519-dalekdata-encodingRelated