Remove warnings from mutable function arguments#1606
Merged
leighmcculloch merged 6 commits intomainfrom Oct 29, 2025
Merged
Conversation
4 tasks
leighmcculloch
changed the title
dmkozh
approved these changes
Oct 29, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 29, 2025
### What Allow contract functions parameters to be immutable references, rather than only owned values. ### Why This change is primarily about making it more convenient to write contract functions like typical Rust functions. In Rust function parameters can be of a few variations, they can be an owned value or a reference/borrow, and can be immutable or mutable. Today contract function parameters must be owned and immutable. Reference parameters are useful when needing to pass around a value that the caller would like to retain ownership over. Mutable parameters are helpful when the function will mutate the parameter making the mutated value available to the caller. It is common when writing small composable Rust functions to use ref and mut as needed. When writing contract functions the composability of the functions being written is restricted. Owned values must be used, and this leads to situations where functions must received cloned values. In contracts cloning SDK types when passing them around is not a performance concern since all SDK types are a small integer value. But it is inconvenient and it makes contracts look less like regular Rust code when there is an abundance of `.clone()` for no apparent reason other than the contract functions don't let you. Is this a big issue? No, not particularly. But in general the soroban-sdk tries to let people write Rust how they'd write Rust elsewhere. And this change lifts a limitation that isn't really required. ### Why not This change introduces variance into the trait-compatible interface that implementations can take. For a contract interface that is intended to be standard, one implementation might use all owned values, another might use all refs, and another might use any combination. This doesn't affect a contracts ability to be called because this information does not make it into the exported interface. But if someone was to share a Rust trait to match one implementation, there's a good chance it would fail to match other implementations. Technically variance already exists today. The `env: Env` parameter is already optional, and can already take either an owned `Env` or a ref `&Env`. So the ship already sailed from day one. You could argue that the top-level entry point value in a program should always be owned. Someone has to own every value. The fact that contract functions do not need to own the value is because there's actually an outer function that gets exported that is truly the entry point and that function can own the value. But in some ways that's a bit odd. ### Merging This change can be merged at anytime and can be released in a minor release. It does not change or limit any existing behaviour and only adds new behaviour that is disallowed today. ### Thanks Thanks to @willemneal contributing #1521 that this change is derived. ### Todo - [x] Test the change on soroban-examples. #1591 - [x] Test the change on several complex ecosystem contracts. - [x] Test with #1544 - [x] #1606 --------- Co-authored-by: Willem Wyndham <willem@ahalabs.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Add test coverage for contract functions that use mutable function arguments. Create new test_mutability contract with a calc function that accepts a mutable parameter, demonstrating that the SDK macros correctly handle mut keywords in contract implementations. Remove warnings that the test coverage surfaced.
Why
The SDK's contract macros already support mutable function parameters, which are is a Rust pattern. This test ensures the macro expansion correctly preserves mutability modifiers and validates the generated code compiles and executes correctly. The test surfaced that warnings are generated in this case, and the warnings shouldn't be as the user is writing valid code and the warnings are coming from the generated code.
Close #1605