stellar / rs-soroban-sdk

Rust SDK for Soroban contracts.
Apache License 2.0
118 stars 66 forks source link

Client is not the zero-cost abstraction it hoped to be #1290

Closed leighmcculloch closed 1 month ago

leighmcculloch commented 2 months ago

What version are you using?

21.1.1

Ref: https://github.com/kalepail/passkey-kit/blob/d78c7c403f0489543b3e3a540f029b8b9980316e/contracts/Cargo.lock#L1110-L1111

What did you do?

Reported here that when importing the client the size of the wasm was 184 bytes larger than when not importing it:

Changing to

mod wallet {
    use soroban_sdk::auth::Context;
    soroban_sdk::contractimport!(file = "../target/wasm32-unknown-unknown/release/webauthn_secp256r1.wasm");
}

and

wallet::Client::new(&env, &address).add(&id, &pk, &true);

Takes the contract from 1209 bytes to 1393.

Ref: https://github.com/stellar/stellar-protocol/discussions/1499#discussioncomment-10001935

What did you expect to see?

Much less change in size. While the compiler will behave differently with different inputs and so we can't guarantee a truly zero cost to using the client the client was designed to cost no more than not using the client.

What did you see instead?

184 byte change in size.

cc @kalepail @willemneal

leighmcculloch commented 1 month ago

I've confirmed that in a minimal test that using a generated client does appear to increase the size of a contract and that the issue is related to the generated client code, and specifically with the use of a short symbol that is being converted to a Val at runtime instead of at compile time, which is pulling in the symbol conversion code.

This is the third (?) time that we've had this specific issue where code has accidentally pulled in the symbol conversion logic.

I've got this on my list to investigate a solution for. On the surface the solution is trivial, but it requires modifying macros, and possibly the common environment code.