manuelmauro / algonaut Goto Github PK

Reference: https://github.com/algorand/java-algorand-sdk/tree/840cf26043f475e43938c64fbda4526a874c258f

As the test suites across official SDKs seem not to be identical, we should stick to one for now. The commit hash is useful as reference, until we come up with a better way to sync with the official SDKs.

The goal is a 1:1 port. When it's not possible, document clearly why (see e.g. these Tests). This might help to sync better with the official SDKs in the future. If the tests need to be in the implementation files, add a note in the external test file.

If it's not possible to implement a test because the functionality isn't supported yet, use #[ignore], write a TODO and open an issue.

• Account
• Client
• Address
• LogicSig
• MultisigAddress
• TransactionSteps
• Applications
• Clients
• EvalDelta
• Indexer
• StepDefs
• TestLogic
• TestMnemonic
• Templates
• Transaction
• AlgodResponses
• IndexerResponses
• PathsShared
• ResponsesShared
• TealSign

https://developer.algorand.org/docs/reference/transactions/

Is your feature request related to a problem? Please describe.
I use algonaut in a project, but specifically only use algod, I would like to not need to compile kmd and indexer related code. This isn't a huge deal for the most part, it's just that all code has to be compiled.

Describe the solution you'd like
add the following features to the algonaut_model, algonaut_client and algonaut: "kmd", "indexer", "algod", (and add them to default features if that is desirable, presumably it is).

#[cfg(feature = "<appropriate feature>")] for kmd, indexer, and algod modules across the aforementioned crates

Sadly this is a breaking change (for default-features = false users at the least) so it needs to be accompanied by a minor version bump (0.x -> 0.y)

Describe alternatives you've considered

1. Keep with the status quo.
2. Only add some of those features but not others
3. Add no-* features. This is considered non-idiomatic, but has the advantage of being non-breaking.

Additional context
N/A

Minimal example project: https://github.com/ivanschuetz/algorand-yew-example

Release version .wasm file size (trunk build --release):

• Without Algonaut, without optimization: 207 KB
• Without Algonaut, with optimization: 143 KB
• With Algonaut, without optimization: 967 KB
• With Algonaut, with optimization: 770 KB

Where the optimization is:

[profile.release]
# less code to include into binary
panic = 'abort' 
# optimization over all codebase ( better optimization, slower build )
codegen-units = 1
# optimization for size ( more aggressive )
opt-level = 'z' 
# optimization for size 
# opt-level = 's' 
# link time optimization using using whole-program analysis
lto = true

(See also https://yew.rs/advanced-topics/optimizations#cargotoml)

These file sizes are obviously not ideal. E.g. building with npm/wasm-pack shows this warning:

WARNING in asset size limit: The following asset(s) exceed the recommended size limit (244 KiB).

We should inspect which of our dependencies are responsible for this and see how we can improve it, in worst case and possible, replacing them with leaner (or web specific) dependencies. ring could be a good place to start, which is causing problems with WASM anyway (#59).

https://developer.algorand.org/docs/reference/rest-apis/indexer/

snd (Sender) has different documentation, depending on whether it's used for a clawback transaction or other transaction types: https://developer.algorand.org/docs/reference/transactions/#asset-clawback-transaction, https://developer.algorand.org/docs/reference/transactions/#common-fields-header-and-type

Allowing users to set snd in both Transaction and AssetClawbackTransaction at the same time seems confusing. Removing it from AssetClawbackTransaction loses the clawback-specific documentation. Solutions that come to mind:

• Explain the 2 possible meanings in the the Transaction's snd (bit messy)
• Remove snd from Transaction and add it to each transaction type with the correct documentation. This would diverge from the way the portal's documentation is structured but it seems correct, as snd as documented in Transaction, is not a common field (the clawback sender doesn't "pay an amount" but withdraws it from one address).

The current builders are minimal implementations of the pattern that do not apply any compile-time validation. This crate offers a solution to this problem https://crates.io/crates/typed-builder while suffering from some limitations.

Is your feature request related to a problem? Please describe.
Not a problem as much as a minor annoyance. The Rust ecosystem tends to like Display impls, and they come with ToString impls.

Describe the solution you'd like
replace the ToString impl of Address with a Display impl.

Describe alternatives you've considered
"not doing that"

Additional context
I'm trying to use a macro from serde_with crate to automatically {de,}serialize an address from/to a string it specifically wants the FromStr and Display impls (TryFrom/Into would work as well), this can be worked around, and isn't really a bug as much as "something I wish existed"

Note that this isn't a breaking change (you still have a ToString impl that is byte-for-byte identical)

Currently the error messages received from the APIs are lost and only the status code is kept. Improve the experience returning additional information.

I'm following along with this guide: https://developer.algorand.org/docs/features/asc1/stateless/sdks/

I'm at the point where I want to be able to provide arguments to the contract. In order to do this I need to generate a LogicSig. This is the example provided for the Go SDK:

// program, err :=  base64.StdEncoding.DecodeString("ASABACI=")
program, err :=  base64.StdEncoding.DecodeString(response.Result)   
var sk ed25519.PrivateKey
var ma crypto.MultisigAccount
var args [][]byte
lsig, err := crypto.MakeLogicSig(program, args, sk, ma)

The other official SDKs (java, python, js) all have similar APIs for creating these signatures.

I've taken a look through the various algonaut crates and examples and haven't found anything specific to creating LogicSigs.

It is possible to create these signatures with algonaut currently?

Describe the bug
When making a payment of 0 ALGO the transaction always fails for signature validation failed.

To Reproduce
Modify the payment example to have a transaction of 0 MicroAlgos:

diff --git a/examples/payment.rs b/examples/payment.rs
index d1a643e..cbb2ba6 100644
--- a/examples/payment.rs
+++ b/examples/payment.rs
@@ -23,7 +23,7 @@ async fn main() -> Result<(), Box<dyn Error>> {
 
     let t = TxnBuilder::with(
         params,
-        Pay::new(from.address(), to.address(), MicroAlgos(123_456)).build(),
+        Pay::new(from.address(), to.address(), MicroAlgos(0)).build(),
     )
     .build();

When the example is run, this is the result I get:

$ ALGOD_URL="https://testnet.algoexplorerapi.io/" ALGOD_TOKEN="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" cargo run --example payment
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
     Running `/tmp/algonaut/target/debug/examples/payment`
response: Err(Request(RequestError { url: Some("https://testnet.algoexplorerapi.io/v2/transactions"), details: Http { status: 400, message: "transaction {_struct:{} Sig:[159 69 199 102 205 5 228 188 220 224 167 132 46 112 72 78 216 211 237 218 59 205 31 140 3 33 176 250 114 165 244 105 17 149 200 84 148 43 47 125 121 37 11 58 201 52 166 74 87 192 222 255 255 65 39 190 203 60 194 129 54 241 144 12] Msig:{_struct:{} Version:0 Threshold:0 Subsigs:[]} Lsig:{_struct:{} Logic:[] Sig:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] Msig:{_struct:{} Version:0 Threshold:0 Subsigs:[]} Args:[]} Txn:{_struct:{} Type:pay Header:{_struct:{} Sender:GIZTTA56FAJNAN7ACK3T6YG34FH32ETDULBZ6ENC4UV7EEHPXJGGSPCMVU Fee:{Raw:1000} FirstValid:15841715 LastValid:15842715 Note:[] GenesisID:testnet-v1.0 GenesisHash:JBR3KGFEWPEE5SAQ6IWU6EEBZMHXD4CZU6WCBXWGF57XBZIJHIRA Group:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA Lease:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] RekeyTo:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ} KeyregTxnFields:{_struct:{} VotePK:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] SelectionPK:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] VoteFirst:0 VoteLast:0 VoteKeyDilution:0 Nonparticipation:false} PaymentTxnFields:{_struct:{} Receiver:BFRTECKTOOE7A5LHCF3TTEOH2A7BW46IYT2SX5VP6ANKEXHZYJY77SJTVM Amount:{Raw:0} CloseRemainderTo:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ} AssetConfigTxnFields:{_struct:{} ConfigAsset:0 AssetParams:{_struct:{} Total:0 Decimals:0 DefaultFrozen:false UnitName: AssetName: URL: MetadataHash:[0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] Manager:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ Reserve:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ Freeze:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ Clawback:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ}} AssetTransferTxnFields:{_struct:{} XferAsset:0 AssetAmount:0 AssetSender:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ AssetReceiver:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ AssetCloseTo:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ} AssetFreezeTxnFields:{_struct:{} FreezeAccount:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ FreezeAsset:0 AssetFrozen:false} ApplicationCallTxnFields:{_struct:{} ApplicationID:0 OnCompletion:NoOpOC ApplicationArgs:[] Accounts:[] ForeignApps:[] ForeignAssets:[] LocalStateSchema:{_struct:{} NumUint:0 NumByteSlice:0} GlobalStateSchema:{_struct:{} NumUint:0 NumByteSlice:0} ApprovalProgram:[] ClearStateProgram:[] ExtraProgramPages:0} CompactCertTxnFields:{_struct:{} CertRound:0 CertType:0 Cert:{_struct:{} SigCommit:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA SignedWeight:0 SigProofs:[] PartProofs:[] Reveals:map[]}}} AuthAddr:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ} invalid : signature validation failed" } }))

Expected behavior
Algonaut should produce a valid signature, since other SDKs (I tried with py-algorand-sdk) do.

We created an abstraction layer for algod/kmd/indexer with an unified error type: AlgonautError.

The are parts of the public API which still use package specific errors like TransactionError. This seems awkward, as AlgonautError suggests that it's the only public error type. It may also lead to confusion in practice, e.g. users write a function that returns AlgonautError (it can be internal convenience around Algonaut, where they doesn't want to convert to other error types yet) and applying ? to the package specific errors will unexpectedly not work.

It may appear that we should have all the public APIs return AlgonautError, but this doesn't seem right, as users would have to unnecessarily handle errors unrelated to the functions they're calling. We could also add From implementations to convert the package specific errors to AlgonautError, which would solve the ? issue, but not the awkward design.

The current AlgonautError as well as the functionality in the root package correspond basically to "clients with convenience": should we create a new package for this, rename it in "client", "service" or similar (and AlgonautError accordingly) (renaming the REST client functionality in something else if needed)?

https://github.com/randlabs/myalgo-connect

Describe the bug
When submitting a query using QueryAccount the same number of results are returned regardless of what limit is set to.

In this example the first request has no limit requested and the second request attempts to limit the number of results to 1. Irrelevant fields have been omitted.

    let query_one: QueryAccount = QueryAccount {
        limit: None,
    };
    let accounts: Vec<Account> = indexer_client.accounts(&query_one).unwrap().accounts;

    println!("q1: {}", accounts.len());

    let query_two: QueryAccount = QueryAccount {
        limit: Some(1),
    };
    let accounts: Vec<Account> = indexer_client.accounts(&query_two).unwrap().accounts;

    println!("q2: {}", accounts.len());

Which produces these results:

q1: 4
q2: 4

By comparison, here are what the limit requests look like directly from the indexer:

$ curl -s "localhost:8980/v2/accounts" | jq '.accounts|length'
4

$ curl -s "localhost:8980/v2/accounts?limit=1" | jq '.accounts|length'
0

$ curl -s "localhost:8980/v2/accounts?limit=2" | jq '.accounts|length'
1

Full example code here: https://gist.github.com/epequeno/e56788eebb7994394be5e5b5123b8a16

Expected behavior
Setting limit to None returns all results. Setting limit to Some(1) returns 1 result.

Additional context
Indexer results when using limit appear to be off-by-one, where a limit of 1 returns 0 results. This issue has been reported here: algorand/indexer#516

Tested against a sandbox network running:

algod version
8590327808
2.6.0.stable [rel/stable] (commit #8fe22d4e)
go-algorand is licensed with AGPLv3.0
source code available at https://github.com/algorand/go-algorand

Indexer version
2.4.0 compiled at 2021-05-31T22:24:40+0000 from git hash baa014ff63c6638074a04fcea32d4ce647b0dbb8 (modified)

The following endpoints are missing:

POST /v1/multisig/signprogram
POST /v1/program/sign

This could be a good reference: https://dev.to/sharkdp/my-release-checklist-for-rust-programs-1m33

This is a snippet of the current workflow.

    let list_response = kmd.list_wallets()?;
    let wallet_id = match list_response
        .wallets
        .into_iter()
        .find(|wallet| wallet.name == "unencrypted-default-wallet")
    {
        Some(wallet) => wallet.id,
        None => return Err("Wallet not found".into()),
    };
    let init_response = kmd.init_wallet_handle(&wallet_id, "")?;
    let wallet_handle_token = init_response.wallet_handle_token;
    println!("Wallet Handle: {}", wallet_handle_token);

Given the API's behavior on receiver side ("zero value" fields have to be always mapped to "missing key") and some tests I did deserializing "zero value" fields(like on_complete), it can be likely assumed that the API always uses "missing key" to represent "zero value", and that when deserializing we should interpret "missing key" depending on context (are we expecting a "zero value"? then it's a "zero value", otherwise the field really doesn't exist), but we should confirm, either testing on a per-field basis or with a specification of the node's mapping policy / inspecting the code base.

The API documentation is not helpful, as it states to use "zero values", which is incorrect:
https://developer.algorand.org/docs/reference/transactions/
https://developer.algorand.org/docs/reference/teal/specification/#oncomplete

Related documentation issues (to date unaddressed):
algorandfoundation/docs#454
algorandfoundation/docs#415

We also should resolve possible ambiguities around "zero values" by specifying what they are: in some cases it's not obvious, e.g. an Address: is it an empty array or an array with 32 bytes set to 0? Is a struct with all the fields set to "zero value" a "zero value" (I remember that this was the case in the Java SDK)? etc.

Describe the bug
Floats are imprecise due to their format, and should never be used for storing financially sensitive data.

To Reproduce
Not sure about the exact maths but:

let off_by_one = MicroAlgos::from_algos(MicroAlgos(1).to_algos() * 5e4); // this is MicroAlgos(49999), probably

Expected behavior

assert!(MicroAlgos::from_algos(MicroAlgos(1).to_algos() * 5e4) == MicroAlgos(50000))

Additional context
I'd look at num::Rational for this. But not sure how that would impact build sizes. I haven't tried it, been busy.

If the intent is an easy way to print MicroAlgos, a custom implementation of Display would suffice. Not too sure what the conversion is for, actually...

Coming to think of it, from_algos should probably act as FromStr does for malformed inputs.

https://github.com/algorand/algorand-sdk-testing

Depends on #84

• Purestake discontinued v1 almost a year ago
• AlgoExplorer doesn't support it anymore
• Devs using a Rusk community SDK seems more on the "bleeding edge" side of things so should be unlikely to be working with or in teams using an archaic node api.
• As indicated in the readme, our APIs can still be considered WIP, which implies less commitment to maintaining backwards compatibility.

Add documentation to the fields composing BlockHeader struct plus check which are optional and correct the type signature.

While messages are convenient structs encoding HTTP requests and responses models are working structs that are meant to be used from the end user of the SDK.

• Tackle repetition across subpackages
• Create user interfacing error type, which shouldn't depend on things like reqwest or rmp_serde.
• Add needed conversions
• Consider renaming error types in subpackages (currently all are named AlgorandError, which could be confusing)
  etc.

https://dependabot.com, https://www.whitesourcesoftware.com/free-developer-tools/renovate/ ?

Ideally it should be configurable for larger spaced intervals (monthly, biweekly?) and be batched to keep impact on history limited.

Currently Address has two methods doing exact this. Implementing the trait From in both directions may improve the struct's ergonomics.

https://github.com/algorand/java-algorand-sdk/blob/840cf26043f475e43938c64fbda4526a874c258f/src/test/java/com/algorand/algosdk/account/TestAccount.java#L154-L164

Is your feature request related to a problem? Please describe.
It's kind of difficult to get the status code of the error for AlgonautError::RequestError. Namely:

AlgonautError::Request(RequestError { details: RequestErrorDetails::Http { status: 404, ... } , ...})

It's nested fairly deep within the error struct, and I feel like this should be a relatively common usage of the error if people attempt to recover from errors.

Describe the solution you'd like/Describe alternatives you've considered
I'm uncertain about what I actually want to see. So I'd like to talk about it.
I'd probably want a combination of the following ideas:

• Understand 404 errors from the indexer when attempting to fetch information about a transaction and return None.
  • Inflexible, potentially not what's intended for a direct translation of the API. Some people might want 404 to be an error.
• Create a method, similar to diesel's optional method on queries, that will do this instead of changing the function signature.
  • Simple, but highly specialized. Possibly inaccurate.
• Add a method to RequestError to check for 404s.
  • Specific, but I'm not sure any other codes are really worth checking. Will need to revisit the REST API specifications.
• Simply create a method on AlgonautError/RequestError/RequestErrorDetails to return Option<u16> when asking for the HTTP status code.
  • It's mostly a question of granularity between this and the previous entry as well as how much parsing is required.
    I'm sure there are better solutions that what I've thought of, but this is what I have.

So far, I've been dealing with it by creating a function to call instead of the default API's function per project.

Additional context
Possibly relates to #99

EDIT: I'm looking specifically at Indexer::transaction_info.

While trying to fix the deserialization of SignedTransaction (which we get from the API in PendingTransaction) I noticed that some fields have different encodings when the transaction is part of a request or response:

• Address: request: byte array, response: base32 string.
• HashDigest: request: byte array, response: base64 string.
• Signature: request: byte array, response: base64 string.

The easy way to fix it is by writing the serializers and deserializers respectively (serialize into a byte array and expect a base32 or base64 string as input to deserialize), but I'm not sure that this is correct. I'd expect object = deserialize(serialize(object)) to work, and with this mechanism it doesn't.

I'm not sure about alternatives: creating different API objects (in this case a different ApiSignedTransaction for the request and response) "only" to handle different encodings might be overkill. Making the deserialization flexible, by allowing multiple encodings, seems messy so wouldn't see it as an option.

Edit: Maybe we can inject a deserializer for these fields at runtime? The default deserializer would be the inverse of the serializer and we would override it for the API calls. Not sure if possible with serde.

Relatedly, the Java integration tests which we're porting, often use a serialized transaction representation as reference: https://github.com/algorand/java-algorand-sdk/blob/840cf26043f475e43938c64fbda4526a874c258f/src/test/java/com/algorand/algosdk/account/TestAccount.java#L157
Not sure that I like these tests, but they show an inconvenience with the "asymmetric" solution: we can't just serialize transactions with the SDK to use them as inputs for these tests, we have to instead submit them to the network and use the API version (returned by pending transactions) instead. In some cases this also may not be possible, as e.g. an incomplete multisig transaction can't be submitted to the network.

Edit: Also want to note that (from our side at least) this is not related with the msg pack encoding used in the requests vs. the JSON encoding used in the response. Serializing the transactions to JSON instead of msg pack expectedly doesn't change the value's encoding.

See e.g.: https://github.com/manuelmauro/algonaut/blob/v0.2.0/algonaut_transaction/src/builder.rs#L104-L107

We probably should return Result. Or maybe add the required fields to new. The later would verify at compile time, but we lose the field names, so e.g. first_valid and last_valid can be accidentally swapped. A possible solution for this could be to pass a new struct that contains only the required fields.

The current tests are for the most part integration tests (send signed transactions to the network).

We should add Unit Tests for validation, serialization, etc. orienting with the official SDKs, e.g.: https://github.com/algorand/go-algorand-sdk/blob/adb6d4be485483d4b95d5eb7742943dc2f2109e9/crypto/crypto_test.go

Should depend on #23, as some of these tests can be avoided by encoding the requirements in the type system.

Algonaut currently doesn't compile for WASM (wasm32-unknown-unknown target). Problems found so far:

• the getrandom crate needs to enable the "js" feature:

rand = "0.8.3"
getrandom = { version = "0.2.2", features = ["js"] 

see:
https://docs.rs/getrandom/0.2.3/getrandom/#indirect-dependencies
https://docs.rs/getrandom/0.2.3/getrandom/#webassembly-support

• reqwest::blocking is not allowed. We've to use async/await.

ring might cause problems too because of the C dependencies. It might have been addressed: briansmith/ring#657 if it causes problems, a quick fix would be to disable it with conditional compilation, as signing/generating key pairs is likely used only by wallet applications.

Calling functions that use ring in a WASM project fails the build:

Account::generate()
Account::from_mnemonic
Account::from_seed()
Account::generate_sig()

This is wallet functionality, so not an issue when using third parties to sign like My Algo or the mobile wallet.

briansmith/ring#657

After having cleared #23 we can do a refactoring in Account, e.g.:

• Use a domain struct for the logic signatures, as described here.
• Above leads to a refactoring of general signing in Account. Some functions probably can be reduced in scope, like merge_multisig_transactions, which is merging only signatures and not the transactions.

The current process for the construction of a payment transaction makes use of three structs: BaseTransaction, Payment, and Transaction. Simplify the process using a builder pattern and implementing Default trait.

While implementing a feature, I've stumbled upon some awkward parts in the official SDKs (Java and Go at least). This includes public APIs.

I wonder how closely we want to follow the public APIs and implementations? Following strictly at least the public APIs has a couple of benefits, but it's a bit limiting if we want to improve them or make them more idiomatic for Rust.

The official SDKs have a module to do client side validation of smart contracts: https://github.com/algorand/java-algorand-sdk/blob/v1.3.1/src/main/java/com/algorand/algosdk/logic/Logic.java

Port and call e.g. when instantiating logic signatures.

https://github.com/algorand/java-algorand-sdk/blob/840cf26043f475e43938c64fbda4526a874c258f/src/test/java/com/algorand/algosdk/account/TestAccount.java#L194-L203

Serialize and deserialize with rmp_serde, diverse parameters, check that the objects are equal.

Is your feature request related to a problem? Please describe.
As a general rule, so far as I can tell, interfaces should use either an iterator or a slice unless you're taking ownership of the data held within. In this case, since assign_group_id only modifies the data provided, I believe this is a better approach.

Describe the solution you'd like
Replace the parameter type of assign_group_id's (Vec<&mut Transaction>) with &mut [Transaction]

Describe alternatives you've considered
It's possible that this is still kind of restrictive. A more obtuse, but more flexible approach is to take in an impl IntoIterator<Item = &mut Transaction>. Both Vec<&mut Transaction> and &mut [Transaction] (and &[&mut Transaction]) should implement this or have conversion methods readily available.

Again, this is a bit obtuse, though.

Additional context
To recap, I'd like:

pub fn assign_group_id(txns: &mut [Transaction]) -> Result<(), TransactionError> {
    let gid = TxGroup::compute_group_id(txns)?;
    for tx in txns {
        tx.assign_group_id(gid);
    }
    Ok(())
}

(I haven't checked if tweaking compute_group_id would affect any other functions.)
OR

pub fn assign_group_id<'a>(txns: impl IntoIterator<Item = &'a mut [Transaction]>) -> Result<(), TransactionError> {
    let txns: Vec<_> = txns.into_iter().collect();
    let gid = TxGroup::compute_group_id(txns.as_slice())?;
    for tx in txns {
        tx.assign_group_id(gid);
    }
    Ok(())
}

OR

pub fn assign_group_id<'a>(txns: impl IntoIterator<IntoIter = impl Iterator<Item = &'a mut Transaction> + Clone>) -> Result<(), TransactionError> {
    let txn_iter = txns.into_iter();
    let gid = TxGroup::compute_group_id(txn_iter.clone())?;
    for tx in txn_iter {
        tx.assign_group_id(gid);
    }
    Ok(())
}

(I... don't think I can recommend this last one for a general purpose interface. The Clone potentially hides a lot of work. Maybe a Copy might be better? You can't just pass a reference since the iterator will get consumed.)

EDIT: Changed the IntoIterator<Item = &'a mut [Transaction]> into IntoIterator<Item = &'a mut Transaction>.

This mostly affect Display and Debug traits.

Currently many structs in the client libraries (algod, kmd, indexer) serialize None fields. Add the proper macro directive from serde to such fields, checking the official Algorand documentation, adding and running tests.

#[serde(skip_serializing_if = "Option::is_none")]

There are parsing errors, in some places there seems to be unnecessary autogenerated code (e.g. usage of Box in models), and the API is unpolished.

Describe the bug
Transactions signed with Account.sign fail validation. The algod backend returns an error:

"message": "transaction {...} invalid : signature validation failed"

Identical transactions singed with the Kmd endpoint don't fail.

To Reproduce

In any of the integration tests, replace

kmd.sign_transaction(&wallet_handle_token, "testpassword", &t)

with

sender_account.sign_transaction(&t)

(and needed adjustments).

Expected behavior
The algod backend should accept the transaction.

PureStake offers a REST API identical to the Algorand node API. This allows the SDKs to use it by just changing the endpoint and API authorization header.

To enable this in our SDK, we have to allow users to pass a custom header.

I'd probably implement this by exposing different builders, one for the node and another for custom endpoints, which allows passing arbitrary headers. This way we don't have to change/complicate the user interfacing node connection API. Internally the clients would be modified to receive all the headers as parameters.

An old version of the API allowed to pass additional headers, which works, but it's not ideal as the builders still expect the user to pass a node token (which is also sent to the PureStake API).

Someone asked a while ago for something to be able to mock data to do frontend work. Should we create traits for these services and maybe create a new package (could be here or in a separate repo) that delivers mock data?

When communicating with off-chain backends or saving to storage, it can make sense to use JSON instead of MessagePack, as it can be more easily inspected. Currently at least Address can't be deserialized from JSON.

A core crate may contain the structs currently in models.rs plus the basic elements of Algorand's protocol, an sdk crate may contain the whole set of API module, and a mnemonic crate may be expanded to support multiple languages or removed in favor of crates already available on crates.io.