Skip to content
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

Add ContractArgs for building function args #1382

Merged
merged 23 commits into from
Nov 1, 2024
Merged

Add ContractArgs for building function args #1382

merged 23 commits into from
Nov 1, 2024

Conversation

leighmcculloch
Copy link
Member

@leighmcculloch leighmcculloch commented Oct 24, 2024

What

Add ContractArgs for building function args.

Usage

-let contract_id = env.register(Contract, (100_u32, 1000_i64));
+let contract_id = env.register(Contract, ContractArgs::__constructor(&100_u32, &1000_i64));

Diff of generated code for constructor test vector

@@ -7,6 +7,8 @@ extern crate core;
 extern crate compiler_builtins as _;
 use soroban_sdk::{contract, contractimpl, contracttype, Env};
 pub struct Contract;
+///ContractArgs is a type for building arg lists for functions defined in "Contract".
+pub struct ContractArgs;
 ///ContractClient is a client for calling the contract defined in "Contract".
 pub struct ContractClient<'a> {
     pub env: soroban_sdk::Env,
@@ -210,6 +212,17 @@ impl<'a> ContractClient<'a> {
         res
     }
 }
+impl ContractArgs {
+    pub fn __constructor<'i>(
+        init_key: &'i u32,
+        init_value: &'i i64,
+    ) -> (&'i u32, &'i i64) {
+        (init_key, init_value)
+    }
+    pub fn get_data<'i>(key: &'i DataKey) -> (&'i DataKey,) {
+        (key,)
+    }
+}
 #[doc(hidden)]
 pub mod ____constructor {
     use super::*;

Why

To provide a way to make specifying args as concrete.

Dependent on:

Close #1348

Note

This is still very much an experiment.

Copy link
Contributor

@dmkozh dmkozh left a comment

Choose a reason for hiding this comment

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

I think this is reasonable. I'm not sure it's too useful outside of constructor args, but in general seems ok to have. It also sort of helps with discoverability, as we can advertise this in all the constructor examples.
The only concern is the wasm size - are we sure the args won't bloat the builds in case if they're not used (but the clients are)?

@leighmcculloch
Copy link
Member Author

The only concern is the wasm size - are we sure the args won't bloat the builds in case if they're not used (but the clients are)?

The tests I ran building for wasm before and after this change resulted in no size change to any of the test vectors. Even when used the new code should be entirely inlined and disappear because it's just data pushing. To reduce the chance we're ever wrong about that I just pushed marking the functions as #[inline(always)].

I should add a test vector that deploys a contract, and uses it, and check that for the deploy case it's also a zero cost or near zero cost construct. I'll do that before marking this ready.

@leighmcculloch
Copy link
Member Author

@dmkozh This is ready for review in conjunction with:

github-merge-queue bot pushed a commit to stellar/rs-soroban-env that referenced this pull request Oct 31, 2024
### What
Fill gaps in ref conversions to Val.

### Why
Needed for:
- stellar/rs-soroban-sdk#1382
@leighmcculloch leighmcculloch marked this pull request as ready for review November 1, 2024 11:24
@leighmcculloch leighmcculloch added this pull request to the merge queue Nov 1, 2024
Merged via the queue into main with commit e59dcc6 Nov 1, 2024
16 checks passed
@leighmcculloch leighmcculloch deleted the i1348 branch November 1, 2024 12:08
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.

Add support for typed constructor calls in tests (register) and deployed code (deploy)
2 participants