[SDK] Introduce a replacement Rust SDK for the entirety of the existi…#20
[SDK] Introduce a replacement Rust SDK for the entirety of the existi…#20gregnazario merged 42 commits intomainfrom
Conversation
…ng one This is based off of the TypeScript SDK, and we'll see how well it goes with more extensive testing.
There was a problem hiding this comment.
Pull request overview
This PR introduces a comprehensive replacement Rust SDK for Aptos (v2), completely redesigned and AI-generated based on the TypeScript SDK. The implementation provides a modern, feature-rich SDK with extensive API clients, account management, cryptographic support, transaction building, and advanced features like retry logic and connection pooling.
Key Changes
- Complete new Rust SDK implementation with modular architecture
- Full API client suite (Fullnode, Faucet, Indexer, ANS)
- Comprehensive account types supporting multiple signature schemes (Ed25519, Secp256k1, Secp256r1, Multi-key, Keyless)
- Transaction building with batching, simulation, and sponsored transaction support
- Advanced features including automatic retry with exponential backoff and connection pooling
Reviewed changes
Copilot reviewed 67 out of 110 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/aptos-rust-sdk-v2/src/api/response.rs | API response types with header metadata extraction and helper methods |
| crates/aptos-rust-sdk-v2/src/api/mod.rs | Module organization for API clients with feature gating |
| crates/aptos-rust-sdk-v2/src/api/indexer.rs | GraphQL indexer client for querying indexed blockchain data |
| crates/aptos-rust-sdk-v2/src/api/fullnode.rs | REST API client for fullnode operations with retry support |
| crates/aptos-rust-sdk-v2/src/api/faucet.rs | Faucet client for testnet account funding |
| crates/aptos-rust-sdk-v2/src/api/ans.rs | Aptos Names Service client for name resolution |
| crates/aptos-rust-sdk-v2/src/account/*.rs | Account implementations for various signature schemes |
| crates/aptos-rust-sdk-v2/scripts/coverage.sh | Test coverage script with multiple profiles |
| crates/aptos-rust-sdk-v2/feature-plans/*.md | Design documentation for implemented features |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 120 out of 232 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 120 out of 232 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 120 out of 232 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| match resource_result { | ||
| Ok(_) => println!(" APT CoinStore already registered"), | ||
| Err(_) => { | ||
| // Register the coin store | ||
| let payload = EntryFunction::new( | ||
| MoveModuleId::from_str_strict("0x1::managed_coin")?, | ||
| "register", | ||
| vec![TypeTag::aptos_coin()], | ||
| vec![], // No arguments besides type arg | ||
| ); |
There was a problem hiding this comment.
Treating any error from get_account_resource as “not registered” can cause incorrect behavior (e.g., transient network/HTTP errors will trigger an unnecessary register attempt). Prefer matching specifically on a “not found” condition (like the pattern used in deploy_module.rs with Err(e) if e.is_not_found()) and otherwise surface/log the real error.
| #[proc_macro_derive(MoveStruct, attributes(move_struct))] | ||
| pub fn derive_move_struct(input: TokenStream) -> TokenStream { | ||
| let input = parse_macro_input!(input as syn::DeriveInput); |
There was a problem hiding this comment.
A new public-facing derive macro (MoveStruct) is introduced here, but there’s no accompanying compile-time test in this PR. Since the crate already has trybuild as a dev-dependency, add a small trybuild pass test that derives MoveStruct and asserts the generated methods type-check (and a fail test for invalid/missing attributes if applicable).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 119 out of 232 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 119 out of 232 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Check formatting | ||
| run: cargo fmt --all -- --check |
There was a problem hiding this comment.
cargo fmt requires the rustfmt component, but the workflow only installs clippy. Add rustfmt to the toolchain components (e.g., components: clippy, rustfmt) to avoid format-check failures in CI.
| if attr.path().is_ident("move_struct") { | ||
| let _ = attr.parse_nested_meta(|meta| { | ||
| if meta.path.is_ident("address") { | ||
| let value: LitStr = meta.value()?.parse()?; | ||
| address = Some(value.value()); | ||
| } else if meta.path.is_ident("module") { | ||
| let value: LitStr = meta.value()?.parse()?; | ||
| module = Some(value.value()); | ||
| } else if meta.path.is_ident("name") { | ||
| let value: LitStr = meta.value()?.parse()?; | ||
| struct_name = Some(value.value()); | ||
| } | ||
| Ok(()) | ||
| }); | ||
| } |
There was a problem hiding this comment.
The result of parse_nested_meta(...) is ignored (let _ = ...), so malformed #[move_struct(...)] attributes won’t produce a compile error and will silently fall back to defaults (e.g., module=unknown). Propagate parse errors by handling the Result and returning to_compile_error() so users get actionable diagnostics.
- Add rustfmt component to release.yml workflow - Fix deploy_module.rs BCS encoding comment (clarify bytes are passed directly) - Fix multi_key_account.rs to use chain_id().id() and propagate errors - Fix call_contract.rs to check specifically for "not found" errors - Add clearer comment in multi_agent.rs about example limitations These changes address the copilot-pull-request-reviewer comments from PR #20.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 119 out of 232 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for attr in &input.attrs { | ||
| if attr.path().is_ident("move_struct") { | ||
| let _ = attr.parse_nested_meta(|meta| { | ||
| if meta.path.is_ident("address") { | ||
| let value: LitStr = meta.value()?.parse()?; | ||
| address = Some(value.value()); | ||
| } else if meta.path.is_ident("module") { | ||
| let value: LitStr = meta.value()?.parse()?; | ||
| module = Some(value.value()); | ||
| } else if meta.path.is_ident("name") { | ||
| let value: LitStr = meta.value()?.parse()?; | ||
| struct_name = Some(value.value()); | ||
| } | ||
| Ok(()) | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Attribute parse errors are being ignored (let _ = ...). This means malformed #[move_struct(...)] attributes can silently produce defaulted values and confusing generated code. Recommend propagating parse errors by returning syn::Error::to_compile_error() so invalid attributes fail compilation with a clear message.
.github/workflows/e2e.yml
Outdated
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Install Rust | ||
| uses: dtolnay/rust-toolchain@stable |
There was a problem hiding this comment.
Same as the release workflow: installing stable can cause CI drift relative to the repo MSRV/toolchain. Pin to the repo toolchain (e.g., 1.90.0) or ensure the action uses the rust-toolchain.toml toolchain for consistent E2E builds.
| uses: dtolnay/rust-toolchain@stable | |
| uses: dtolnay/rust-toolchain@master |
| @@ -0,0 +1,426 @@ | |||
| //! Code generation for contract bindings. | |||
|
|
|||
There was a problem hiding this comment.
Looks like the view function code you get from the macro feature return just serde_json::Value rather than a typed struct, we should improve that.
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| pub struct MoveModuleABI { | ||
| /// The module address. | ||
| pub address: String, |
There was a problem hiding this comment.
We need a way to override the account address in the ABI when building txn payloads and view functions and the like (from the macro generated code). I usually deploy contracts to different addresses on different networks, so I'd rather not rely on the value from the ABI.
|
|
||
| /// The fungible asset framework address (0x4). | ||
| pub const FOUR: Self = Self::from_u64(4); | ||
|
|
There was a problem hiding this comment.
Let's have A here too, for the APT fungible asset metadata address.
| pub fn to_bytes(&self) -> [u8; ADDRESS_LENGTH] { | ||
| self.0 | ||
| } | ||
|
|
There was a problem hiding this comment.
The Display impl should be the AIP-40 representation (LONG for normal account addresses, SHORT for special addresses). Then there should be to_short_string and to_long_string functions.
See https://github.com/aptos-foundation/AIPs/blob/main/aips/aip-40.md.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 123 out of 242 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| edition = "2024" | ||
| homepage = "https://aptoslabs.com" | ||
| license = "Apache-2.0" | ||
| publish = false |
There was a problem hiding this comment.
The workspace sets publish = false, which is inherited by member crates unless overridden. This will cause the new release workflow’s cargo publish step to fail for aptos-rust-sdk-v2 (and the macros crate, if you intend to publish it). If publishing is intended, set publish = true explicitly in the crate(s) that should be publishable (e.g. crates/aptos-rust-sdk-v2/Cargo.toml, and crates/aptos-rust-sdk-v2-macros/Cargo.toml), or adjust the workflow to avoid publishing.
| publish = false | |
| publish = true |
| async fn check_account_exists(aptos: &Aptos, address: AccountAddress) -> bool { | ||
| aptos.get_sequence_number(address).await.is_ok() |
There was a problem hiding this comment.
This treats any error (e.g., transient network failures, rate limiting) as “account does not exist”, which can produce incorrect results. Consider returning anyhow::Result<bool> and only mapping “not found” into Ok(false) while propagating other errors, or performing a request that has an explicit “not found” classification and branching on e.is_not_found().
| async fn check_account_exists(aptos: &Aptos, address: AccountAddress) -> bool { | |
| aptos.get_sequence_number(address).await.is_ok() | |
| async fn check_account_exists(aptos: &Aptos, address: AccountAddress) -> anyhow::Result<bool> { | |
| match aptos.get_sequence_number(address).await { | |
| Ok(_) => Ok(true), | |
| Err(e) if e.is_not_found() => Ok(false), | |
| Err(e) => Err(e.into()), | |
| } |
| // Get the private key bytes and convert to hex | ||
| let pk_bytes = random_account.private_key().to_bytes(); | ||
| let pk_hex = hex::encode(pk_bytes); | ||
| println!("Private key (hex): {}", pk_hex); |
There was a problem hiding this comment.
Printing private keys in examples is easy for users to copy-paste into real usage and can lead to accidental key disclosure via terminal logs/CI logs/shell history. Consider either removing the private key print, masking it (e.g., show only prefix/suffix), or gating it behind an explicit environment variable (e.g., SHOW_PRIVATE_KEY=1) while keeping the restoration example.
| /// ``` | ||
| #[proc_macro] | ||
| pub fn aptos_contract(input: TokenStream) -> TokenStream { |
There was a problem hiding this comment.
The proc-macro surface (aptos_contract!, aptos_contract_file!, and #[derive(MoveStruct)]) introduces non-trivial parsing and error-reporting behavior, but this diff doesn’t include any compile-fail/trybuild coverage for invalid ABI JSON, missing files, or malformed #[move_struct(...)] attributes. Adding trybuild tests for both success and failure cases would help keep diagnostics stable and prevent silent parsing regressions.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 122 out of 242 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let payload = MyCoin::transfer(recipient, amount)?; | ||
| let balance = MyCoin::view_balance(&aptos, owner).await?; |
There was a problem hiding this comment.
The README shows calling generated methods as associated functions (MyCoin::transfer, MyCoin::view_balance), but the same README later documents the generated code as instance methods on &self. Update the usage example to match the actual generated API shape (either consistently instance methods, or consistently associated fns) to avoid confusing users.
| let payload = MyCoin::transfer(recipient, amount)?; | |
| let balance = MyCoin::view_balance(&aptos, owner).await?; | |
| let my_coin = /* obtain a `MyCoin` client instance */; | |
| let payload = my_coin.transfer(recipient, amount)?; | |
| let balance = my_coin.view_balance(owner).await?; |
| println!("Private key (hex): {}", pk_hex); | ||
|
|
There was a problem hiding this comment.
This example prints the raw private key to stdout, which is easy for users to copy/paste into logs or CI artifacts accidentally. Consider either removing the private-key print entirely, redacting it (e.g., show only prefix/suffix), or gating it behind an explicit opt-in (like an environment variable) while still demonstrating from_private_key_hex.
| println!("Private key (hex): {}", pk_hex); | |
| // For safety, avoid printing the full private key. Show only a redacted version. | |
| let redacted_pk = if pk_hex.len() > 16 { | |
| format!("{}...{}", &pk_hex[..8], &pk_hex[pk_hex.len() - 8..]) | |
| } else { | |
| "[redacted]".to_string() | |
| }; | |
| println!("Private key (hex, redacted): {}", redacted_pk); |
| name: Publish to crates.io | ||
| runs-on: ubuntu-latest | ||
| needs: verify | ||
| if: startsWith(github.ref, 'refs/tags/v') |
There was a problem hiding this comment.
The workflow declares a workflow_dispatch input (dry_run), but the publish job is gated only on startsWith(github.ref, 'refs/tags/v'). For manual runs on a branch (a common way to validate cargo publish --dry-run before tagging), the job will never execute. Consider updating the job-level if to also allow github.event_name == 'workflow_dispatch' (and then ensure the steps use --dry-run accordingly).
| if: startsWith(github.ref, 'refs/tags/v') | |
| if: startsWith(github.ref, 'refs/tags/v') || github.event_name == 'workflow_dispatch' |
- actions/checkout v4 -> v6 - actions/configure-pages v4 -> v5 - actions/upload-pages-artifact v3 -> v4 - softprops/action-gh-release v1 -> v2
Use single braces for import statement in the printed code example. The code is inside a raw string (not a format string), so double braces render literally instead of being escaped.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 122 out of 242 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn format_apt(octas: u64) -> String { | ||
| let apt = octas as f64 / OCTAS_PER_APT; | ||
| if apt >= 1.0 { | ||
| format!("{:.4}", apt) | ||
| } else if apt >= 0.0001 { | ||
| format!("{:.6}", apt) | ||
| } else { | ||
| format!("{:.8}", apt) | ||
| } | ||
| } |
There was a problem hiding this comment.
Converting u64 octas to f64 can lose precision for large balances (f64 has ~53 bits of integer precision), so the displayed APT amount may be inaccurate. Prefer integer-based formatting (split into whole/octas via division/modulo with OCTAS_PER_APT as u64) or use a decimal type for exact display.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 134 out of 242 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| println!(" {}", mnemonic_phrase); | ||
| println!("\n ⚠️ IMPORTANT: Store this phrase securely!"); |
There was a problem hiding this comment.
This example prints the full mnemonic phrase and a private key to stdout. Even with warnings, this pattern is easy to cargo-run in shared terminals/CI logs and can leak secrets. Prefer not printing secrets at all, or gate printing behind an explicit opt-in (e.g., PRINT_SECRETS=1) and otherwise print only public information (address/public key) or a redacted/truncated preview.
| println!(" {}", mnemonic_phrase); | |
| println!("\n ⚠️ IMPORTANT: Store this phrase securely!"); | |
| let print_secrets = std::env::var("PRINT_SECRETS") | |
| .map(|v| v == "1" || v.eq_ignore_ascii_case("true")) | |
| .unwrap_or(false); | |
| if print_secrets { | |
| println!(" {}", mnemonic_phrase); | |
| println!("\n ⚠️ IMPORTANT: Store this phrase securely!"); | |
| } else { | |
| println!(" [hidden]"); | |
| println!("\n ⚠️ IMPORTANT: This mnemonic gives full control over your funds."); | |
| println!(" It is hidden by default. Set PRINT_SECRETS=1 to display it in this example."); | |
| } |
- Add security.yml workflow with cargo-audit and cargo-deny checks - Runs on push, PR, and daily schedule to catch new advisories - Update bytes crate 1.11.0 -> 1.11.1 to fix RUSTSEC-2026-0007 (integer overflow in BytesMut::reserve)
…ng one
This is based off of the TypeScript SDK, and we'll see how well it goes with more extensive testing.
This is completely AI generated but driven by some specs