Skip to content

feat: algokit_utils (Rust impl only) #166

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.

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

Merged
merged 55 commits into from
Jun 25, 2025
Merged

feat: algokit_utils (Rust impl only) #166

merged 55 commits into from
Jun 25, 2025

Conversation

joe-p
Copy link
Collaborator

@joe-p joe-p commented Jun 20, 2025

Follows up on the spike in #119 but removes the FFI utils crate and packages for now. When working on them I realized that they will take a decent amount of time to implement due do all the new types/traits, so figured it makes more sense to review the rust impl on its own and then later PR the FFI crate and packages

joe-p added 30 commits June 3, 2025 11:29
Had to work around this bug in maturin:
PyO3/maturin#2459

This fix is relatively simple but requires you to modify the packages
post-installation. A fix (more like feature) can be added to maturin and
I think it should be fairly straightforward
There is something weird going on with the env package though. For now
you need to bun rm env, run the build script, and then bun i env before
running the tests
Mutex works in WASM and only adds a few nanos of overhead, so just
easier to use Mutex for both uniffi and wasm
It works, but there seems to be some closure that isn't getting cleaned
up properly in the async call
Bun prefers main over module for some reason at runtime. See
oven-sh/bun#13430 (comment)
@joe-p joe-p marked this pull request as ready for review June 20, 2025 19:33
@Copilot Copilot AI review requested due to automatic review settings June 20, 2025 19:33
Copilot

This comment was marked as outdated.

@joe-p joe-p requested review from Copilot and neilcampbell June 25, 2025 15:00
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a pure Rust implementation of the algokit_utils crate for transaction composition and signing, introduces an ffi_mutex crate for FFI-safe interior mutability, and updates workspace and crate configurations for FFI and HTTP client support.

  • Introduces ffi_mutex crate with a unified mutex/refcell wrapper for Uniffi and WASM.
  • Implements algokit_utils with a Composer for building and signing Algorand transactions.
  • Updates workspace members and adjusts crate-types/features in FFI and HTTP client crates.

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crates/ffi_mutex/src/lib.rs Adds FfiMutex wrapper selecting tokio::Mutex or RefCell by feature
crates/ffi_mutex/Cargo.toml Defines features and optional dependencies for the FFI mutex crate
crates/algokit_utils/src/lib.rs Implements Composer, transaction building, grouping, and signature logic
crates/algokit_utils/Cargo.toml Configures features and dependencies for the algokit_utils crate
crates/algokit_transact_ffi/Cargo.toml Adds "lib" to crate-type list for the FFI binding crate
crates/algokit_http_client/src/lib.rs Defines HttpClient trait implementations for default and WASM
crates/algokit_http_client/Cargo.toml Sets up features and deps for HTTP client crate
crates/algod_api/src/lib.rs Implements AlgodClient wrapper over an HTTP client to fetch params
crates/algod_api/Cargo.toml Configures features and dependencies for the algod_api crate
Cargo.toml Updates workspace members to include new crates (ffi_mutex, algokit_utils, etc.)

Comment on lines +213 to +215
// The rest of these fields are set further down per txn
first_valid: 0,
last_valid: 0,
Copy link
Preview

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

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

The first_valid and last_valid fields are hard-coded to 0. You should initialize them from suggested_params.last_round or common_params.first_valid_round/last_valid_round to ensure transactions have a valid window.

Suggested change
// The rest of these fields are set further down per txn
first_valid: 0,
last_valid: 0,
// Initialize valid round range dynamically
first_valid: common_params.first_valid_round.unwrap_or(suggested_params.last_round),
last_valid: suggested_params.last_round,

Copilot uses AI. Check for mistakes.

pub rekey_to: Option<Address>,
pub note: Option<Vec<u8>>,
pub lease: Option<[u8; 32]>,
pub static_fee: Option<u64>,
Copy link
Preview

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

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

The static_fee (and related validity_window, first_valid_round, last_valid_round) fields on CommonParams are never used when building transactions. Consider applying static_fee to override the calculated fee and use the validity fields when setting first_valid/last_valid.

Copilot uses AI. Check for mistakes.

let result = self.get(&path).await.map_err(|e| {
HttpError::HttpError(
e.as_string().unwrap_or(
"A HTTP error ocurred in JavaScript, but it cannot be converted to a string"
Copy link
Preview

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

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

Fix spelling: change "ocurred" to "occurred" in the error message.

Suggested change
"A HTTP error ocurred in JavaScript, but it cannot be converted to a string"
"A HTTP error occurred in JavaScript, but it cannot be converted to a string"

Copilot uses AI. Check for mistakes.

}

/// A temporary AlgodClient until the proper client is generated.
/// The exepectation is that this client will use a HttpClient to make requests to the Algorand API
Copy link
Preview

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

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

Fix spelling: change "exepectation" to "expectation" in the documentation comment.

Suggested change
/// The exepectation is that this client will use a HttpClient to make requests to the Algorand API
/// The expectation is that this client will use a HttpClient to make requests to the Algorand API

Copilot uses AI. Check for mistakes.

@joe-p
Copy link
Collaborator Author

joe-p commented Jun 25, 2025

Will want to loop back and take a closer look at those copilot comments, but going ahead and merging as is to unblock everyone else adding new txn types to composer

@joe-p joe-p merged commit 905e072 into main Jun 25, 2025
21 checks passed
@joe-p joe-p deleted the feat/rust_utils branch June 25, 2025 17:55
@engineering-ci
Copy link
Contributor

🎉 This PR is included in version 1.0.0-alpha.22 🎉

The release is available on:

Your semantic-release bot 📦🚀

@engineering-ci
Copy link
Contributor

🎉 This PR is included in version 1.0.0-alpha.20 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@engineering-ci
Copy link
Contributor

🎉 This PR is included in version 1.0.0-alpha.19 🎉

The release is available on:

Your semantic-release bot 📦🚀

@engineering-ci
Copy link
Contributor

🎉 This PR is included in version 1.0.0-alpha.21 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants