Skip to content

Commit

Permalink
Add register and register_at fns to replace register_contract, …
Browse files Browse the repository at this point in the history
…`register_contract_with_constructor`, `register_contract_wasm`, `register_contract_wasm_with_constructor` (#1343)

### What

Add `register` and `register_at` fns.

Deprecate `register_contract` and `register_contract_wasm`.

Unexport `register_contract_with_constructor` and
`register_contract_wasm_with_constructor`.

### Why

To reorient the register functions around what we expect the common
cases to be:
 - registering without a contract id specified
 - registering with a constructor specified

It's always been odd that the first parameter to most
`register_contract` calls is `None`. It's confusing to look at the first
time you see it, and for the most part most developers do not need to
set a value for the contract id.

Most contracts need initialization, and therefore I think most contracts
over time will gain constructors.

To do so in a way that is backwards compatible with previous releases.

Close #1341

### Known limitations

It would be better imo if we could add the constructor arguments to the
existing `register_contract` and `register_contract_wasm` fns, but that
would be a breaking change.

The `deploy_with_constructor` functions still exist which is
inconsistent, but harder to align because to change the `deploy`
function to always accept constructor args would be a breaking change.

It's possible to update code that uses the existing register functions
using a regex find and replace, but it's not the most trivial regex to
work out. If folks wish to resolve the deprecation warning they'll need
to update potentially every test they've written. That in itself is not
a great developer experience.

The old and new way don't look that different, and so for existing
Soroban developers, the change from the old to new way may be confusing
at a glance.

This PR doesn't touch the register stellar asset contract functions at
all. Ideas?
  • Loading branch information
leighmcculloch authored Sep 24, 2024
1 parent 1367be1 commit 9507fc4
Show file tree
Hide file tree
Showing 58 changed files with 344 additions and 219 deletions.
2 changes: 1 addition & 1 deletion soroban-sdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ fn test() {
# #[cfg(feature = "testutils")]
# fn main() {
let env = Env::default();
let contract_id = env.register_contract(None, HelloContract);
let contract_id = env.register(HelloContract, ());
let client = HelloContractClient::new(&env, &contract_id);

let words = client.hello(&symbol_short!("Dev"));
Expand Down
4 changes: 2 additions & 2 deletions soroban-sdk/src/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
//! # #[cfg(feature = "testutils")]
//! # fn main() {
//! let env = Env::default();
//! let contract_address = env.register_contract(None, Contract);
//! let contract_address = env.register(Contract, ());
//! let contract = ContractClient::new(&env, &contract_address);
//! // Upload the contract code before deploying its instance.
//! let wasm_hash = env.deployer().upload_contract_wasm(DEPLOYED_WASM);
Expand Down Expand Up @@ -71,7 +71,7 @@
//! # #[cfg(feature = "testutils")]
//! # fn main() {
//! let env = Env::default();
//! let contract_address = env.register_contract(None, Contract);
//! let contract_address = env.register(Contract, ());
//! let contract = ContractClient::new(&env, &contract_address);
//! // Upload the contract code before deploying its instance.
//! let wasm_hash = env.deployer().upload_contract_wasm(DEPLOYED_WASM_WITH_CTOR);
Expand Down
198 changes: 145 additions & 53 deletions soroban-sdk/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ use crate::{
testutils::{
budget::Budget, Address as _, AuthSnapshot, AuthorizedInvocation, ConstructorArgs,
ContractFunctionSet, EventsSnapshot, Generators, Ledger as _, MockAuth, MockAuthContract,
Snapshot, StellarAssetContract, StellarAssetIssuer,
Register, Snapshot, StellarAssetContract, StellarAssetIssuer,
},
Bytes, BytesN,
};
Expand Down Expand Up @@ -576,15 +576,144 @@ impl Env {
env
}

/// Register a contract with the [Env] for testing.
///
/// Pass the contract type when the contract is defined in the current crate
/// and is being registered natively. Pass the contract wasm bytes when the
/// contract has been loaded as wasm.
///
/// Pass the arguments for the contract's constructor, or `()` if none.
///
/// Returns the address of the registered contract that is the same as the
/// contract id passed in.
///
/// If you need to specify the address the contract should be registered at,
/// use [`register_at`].
///
/// ### Examples
/// Register a contract defined in the current crate, by specifying the type
/// name:
/// ```
/// use soroban_sdk::{contract, contractimpl, testutils::Address as _, Address, BytesN, Env, Symbol};
///
/// #[contract]
/// pub struct Contract;
///
/// #[contractimpl]
/// impl Contract {
/// pub fn __constructor(_env: Env, _input: u32) {
/// }
/// }
///
/// #[test]
/// fn test() {
/// # }
/// # fn main() {
/// let env = Env::default();
/// let contract_id = env.register(Contract, (123_u32,));
/// }
/// ```
/// Register a contract wasm, by specifying the wasm bytes:
/// ```
/// use soroban_sdk::{testutils::Address as _, Address, BytesN, Env};
///
/// const WASM: &[u8] = include_bytes!("../doctest_fixtures/contract.wasm");
///
/// #[test]
/// fn test() {
/// # }
/// # fn main() {
/// let env = Env::default();
/// let contract_id = env.register(WASM, ());
/// }
/// ```
pub fn register<'a, C, A>(&self, contract: C, constructor_args: A) -> Address
where
C: Register,
A: ConstructorArgs,
{
contract.register(self, None, constructor_args)
}

/// Register a contract with the [Env] for testing.
///
/// Passing a contract ID for the first arguments registers the contract
/// with that contract ID.
///
/// Registering a contract that is already registered replaces it.
/// Use re-registration with caution as it does not exist in the real
/// (on-chain) environment. Specifically, the new contract's constructor
/// will be called again during re-registration. That behavior only exists
/// for this test utility and is not reproducible on-chain, where contract
/// Wasm updates don't cause constructor to be called.
///
/// Pass the contract type when the contract is defined in the current crate
/// and is being registered natively. Pass the contract wasm bytes when the
/// contract has been loaded as wasm.
///
/// Returns the address of the registered contract that is the same as the
/// contract id passed in.
///
/// ### Examples
/// Register a contract defined in the current crate, by specifying the type
/// name:
/// ```
/// use soroban_sdk::{contract, contractimpl, testutils::Address as _, Address, BytesN, Env, Symbol};
///
/// #[contract]
/// pub struct Contract;
///
/// #[contractimpl]
/// impl Contract {
/// pub fn __constructor(_env: Env, _input: u32) {
/// }
/// }
///
/// #[test]
/// fn test() {
/// # }
/// # fn main() {
/// let env = Env::default();
/// let contract_id = Address::generate(&env);
/// env.register_at(&contract_id, Contract, (123_u32,));
/// }
/// ```
/// Register a contract wasm, by specifying the wasm bytes:
/// ```
/// use soroban_sdk::{testutils::Address as _, Address, BytesN, Env};
///
/// const WASM: &[u8] = include_bytes!("../doctest_fixtures/contract.wasm");
///
/// #[test]
/// fn test() {
/// # }
/// # fn main() {
/// let env = Env::default();
/// let contract_id = Address::generate(&env);
/// env.register_at(&contract_id, WASM, ());
/// }
/// ```
pub fn register_at<C, A>(
&self,
contract_id: &Address,
contract: C,
constructor_args: A,
) -> Address
where
C: Register,
A: ConstructorArgs,
{
contract.register(self, contract_id, constructor_args)
}

/// Register a contract with the [Env] for testing.
///
/// Passing a contract ID for the first arguments registers the contract
/// with that contract ID. Providing `None` causes the Env to generate a new
/// contract ID that is assigned to the contract.
///
/// If a contract has a constructor defined, then it will be called with
/// no arguments. If a constructor takes arguments, use
/// `register_contract_with_constructor`.
/// no arguments. If a constructor takes arguments, use `register`.
///
/// Registering a contract that is already registered replaces it.
/// Use re-registration with caution as it does not exist in the real
Expand Down Expand Up @@ -617,6 +746,7 @@ impl Env {
/// let contract_id = env.register_contract(None, HelloContract);
/// }
/// ```
#[deprecated(note = "use `register`")]
pub fn register_contract<'a, T: ContractFunctionSet + 'static>(
&self,
contract_id: impl Into<Option<&'a Address>>,
Expand All @@ -642,30 +772,7 @@ impl Env {
/// Wasm updates don't cause constructor to be called.
///
/// Returns the address of the registered contract.
///
/// ### Examples
/// ```
/// use soroban_sdk::{contract, contractimpl, BytesN, Env, Symbol};
///
/// #[contract]
/// pub struct Contract;
///
/// #[contractimpl]
/// impl Contract {
/// pub fn __constructor(_env: Env, _input: u32) {
/// }
/// }
///
/// #[test]
/// fn test() {
/// # }
/// # fn main() {
/// let env = Env::default();
/// let contract_id = env.register_contract_with_constructor(
/// None, Contract, (123_u32,));
/// }
/// ```
pub fn register_contract_with_constructor<
pub(crate) fn register_contract_with_constructor<
'a,
T: ContractFunctionSet + 'static,
A: ConstructorArgs,
Expand Down Expand Up @@ -742,6 +849,7 @@ impl Env {
/// env.register_contract_wasm(None, WASM);
/// }
/// ```
#[deprecated(note = "use `register`")]
pub fn register_contract_wasm<'a>(
&self,
contract_id: impl Into<Option<&'a Address>>,
Expand Down Expand Up @@ -772,33 +880,17 @@ impl Env {
/// Wasm updates don't cause constructor to be called.
///
/// Returns the address of the registered contract.
///
/// ### Examples
/// ```
/// use soroban_sdk::{BytesN, Env, IntoVal};
/// // This is Wasm for `constructor` test contract from this repo.
/// const WASM: &[u8] = include_bytes!("../doctest_fixtures/contract_with_constructor.wasm");
///
/// #[test]
/// fn test() {
/// # }
/// # fn main() {
/// let env = Env::default();
/// env.register_contract_wasm_with_constructor(
/// None, WASM, (10_u32, 100_i64).into_val(&env));
/// }
/// ```
pub fn register_contract_wasm_with_constructor<'a>(
pub(crate) fn register_contract_wasm_with_constructor<'a>(
&self,
contract_id: impl Into<Option<&'a Address>>,
contract_wasm: impl IntoVal<Env, Bytes>,
constructor_args: Vec<Val>,
constructor_args: impl ConstructorArgs,
) -> Address {
let wasm_hash: BytesN<32> = self.deployer().upload_contract_wasm(contract_wasm);
self.register_contract_with_optional_contract_id_and_executable(
contract_id,
xdr::ContractExecutable::Wasm(xdr::Hash(wasm_hash.into())),
constructor_args,
constructor_args.into_val(self),
)
}

Expand Down Expand Up @@ -995,7 +1087,7 @@ impl Env {
/// # }
/// # fn main() {
/// let env = Env::default();
/// let contract_id = env.register_contract(None, HelloContract);
/// let contract_id = env.register(HelloContract, ());
///
/// let client = HelloContractClient::new(&env, &contract_id);
/// let addr = Address::generate(&env);
Expand All @@ -1014,7 +1106,7 @@ impl Env {
/// ```
pub fn mock_auths(&self, auths: &[MockAuth]) {
for a in auths {
self.register_contract(a.address, MockAuthContract);
self.register_at(a.address, MockAuthContract, ());
}
let auths = auths
.iter()
Expand Down Expand Up @@ -1064,7 +1156,7 @@ impl Env {
/// # }
/// # fn main() {
/// let env = Env::default();
/// let contract_id = env.register_contract(None, HelloContract);
/// let contract_id = env.register(HelloContract, ());
///
/// env.mock_all_auths();
///
Expand Down Expand Up @@ -1117,8 +1209,8 @@ impl Env {
/// # }
/// # fn main() {
/// let env = Env::default();
/// let contract_a = env.register_contract(None, ContractA);
/// let contract_b = env.register_contract(None, ContractB);
/// let contract_a = env.register(ContractA, ());
/// let contract_b = env.register(ContractB, ());
/// // The regular `env.mock_all_auths()` would result in the call
/// // failure.
/// env.mock_all_auths_allowing_non_root_auth();
Expand Down Expand Up @@ -1178,7 +1270,7 @@ impl Env {
/// # fn main() {
/// extern crate std;
/// let env = Env::default();
/// let contract_id = env.register_contract(None, Contract);
/// let contract_id = env.register(Contract, ());
/// let client = ContractClient::new(&env, &contract_id);
/// env.mock_all_auths();
/// let address = Address::generate(&env);
Expand Down Expand Up @@ -1282,7 +1374,7 @@ impl Env {
/// # }
/// # fn main() {
/// let e: Env = Default::default();
/// let account_contract = NoopAccountContractClient::new(&e, &e.register_contract(None, NoopAccountContract));
/// let account_contract = NoopAccountContractClient::new(&e, &e.register(NoopAccountContract, ()));
/// // Non-succesful call of `__check_auth` with a `contracterror` error.
/// assert_eq!(
/// e.try_invoke_contract_check_auth::<NoopAccountError>(
Expand Down
2 changes: 1 addition & 1 deletion soroban-sdk/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const TOPIC_BYTES_LENGTH_LIMIT: u32 = 32;
/// # #[cfg(feature = "testutils")]
/// # fn main() {
/// # let env = Env::default();
/// # let contract_id = env.register_contract(None, Contract);
/// # let contract_id = env.register(Contract, ());
/// # ContractClient::new(&env, &contract_id).f();
/// # }
/// # #[cfg(not(feature = "testutils"))]
Expand Down
2 changes: 1 addition & 1 deletion soroban-sdk/src/ledger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use crate::{env::internal, unwrap::UnwrapInfallible, BytesN, Env, TryIntoVal};
/// # #[cfg(feature = "testutils")]
/// # fn main() {
/// # let env = Env::default();
/// # let contract_id = env.register_contract(None, Contract);
/// # let contract_id = env.register(Contract, ());
/// # ContractClient::new(&env, &contract_id).f();
/// # }
/// # #[cfg(not(feature = "testutils"))]
Expand Down
Loading

0 comments on commit 9507fc4

Please sign in to comment.