Updated the deployer example. #230

dmkozh · 2022-12-01T22:46:26Z

This is still incomplete:

The examples tag should be bumped
The CLI should be updated, so that there is some way to install the contract WASM

I'm sending this for a preliminary review as the updates above are trivial.

stellar-jenkins · 2022-12-01T22:49:02Z

Preview is available here:
http://soroban-docs-pr230.previews.kube001.services.stellar-ops.com/

This is still incomplete: - The examples tag should be bumped - The CLI should be updated, so that there is some way to install the contract WASM

stellar-jenkins · 2022-12-02T18:20:58Z

Preview is available here:
http://soroban-docs-pr230.previews.kube001.services.stellar-ops.com/

leighmcculloch

Few requests, most minor, the one at the end might be tricky.

leighmcculloch · 2022-12-02T18:20:05Z

docs/examples/deployer.mdx

-Open the `deployer/deployer/src/src.rs` file to follow along.
+Open the `deployer/deployer/src/lib.rs` file to follow along.
+
+### Contract code installation


Title needs capitalizing for consistency, and I suggest we shorten it, since it is easier to create a concept stories around single word names like "installation" and "deployment".

Suggested change

### Contract code installation

### Contract Installation

Capitalized. I think we need to keep 'code' around though, as we are not installing the contract yet (IMO 'contract deploy' and 'contract install' aren't really distinguishable and both concern 'contracts', while the code is not a contract).

It's not really "code" either, it's compiled into some form of code, but not "code" the way developers think of code normally. And it is a contract. It's literally the contract. Deployment creates a contract instance, one of the paragraphs in the doc use that language of "contract instance". So I think we already have dual usage of the term. But that's fine, lets keep code as that's what is in the env and XDR too I realize.

WASM is binary code and that's the way I think about that personally. But I'm fine with updating the terminology to make it less ambiguous. I agree that the code is a contract in some sense, but OTOH saying 'contract instance' every time we refer to the thing you can invoke (and the thing that you'd use 99% of the time) is kind of redundant. Maybe they can be used interchangeably, but then we need something to disambiguate WASM in docs like this one.

docs/examples/deployer.mdx

leighmcculloch · 2022-12-02T18:21:19Z

docs/examples/deployer.mdx

+
+Before deploying the new contract instances, the WASM code needs to be installed
+on-chain. Then it can be reused to deploy an arbitrary number of contract 
+instances. The installation typically should happen outside of the deployer


Suggested change

instances. The installation typically should happen outside of the deployer

instances. The installation should typically happen outside of the deployer

leighmcculloch · 2022-12-02T18:22:21Z

docs/examples/deployer.mdx

+[Below](#tests) is the example of installing the contract code in tests. For the actual
+installation see the general deployment [tutorial](https://soroban.stellar.org/docs/tutorials/deploy-to-futurenet).


Suggested change

[Below](#tests) is the example of installing the contract code in tests. For the actual

installation see the general deployment [tutorial](https://soroban.stellar.org/docs/tutorials/deploy-to-futurenet).

See the [tests](#tests) for an example of installing the contract in tests. For the actual

installation see the deployment [tutorial](https://soroban.stellar.org/docs/tutorials/deploy-to-futurenet).

leighmcculloch · 2022-12-02T18:22:57Z

docs/examples/deployer.mdx


 ### Deployer

 The `deployer` contract deploys other contracts. It accepts a salt that will be
-used to derive a contract ID, and a WASM bytes to deploy. It also accepts an
-initialization function and arguments to pass to that function.
+used to derive a contract ID, and a hash-based identifier of the installed WASM


Suggested change

used to derive a contract ID, and a hash-based identifier of the installed WASM

used to derive a contract ID, and a hash of the installed WASM

Not sure about removing this... Maybe we should really only hash the code (initially the install entries had more fields), but that would need to happen in the next release. Added an additional comment to make that clear.

Is the wasm hash only a hash of the code or something else? Because If it is of something else I think the SDK is misleading.

docs/examples/deployer.mdx

leighmcculloch · 2022-12-02T18:26:28Z

docs/examples/deployer.mdx


 ```rust
 env.register_contract(&deployer_id, Deployer);
 ```

+...and created a client for it:


Suggested change

...and created a client for it:

Create a client that will be used to invoke the contract.

leighmcculloch · 2022-12-02T18:27:22Z

docs/examples/deployer.mdx

-The deployer contract is registered with the environment.
+Register the deployer contract with the environment:


Can you keep these as sentences with trailing . for consistency with the other other examples. I don't mind changing it, but if we're going to change it, can you go over every page in the doc and update them all? The docs try and be as consistent as possible.

leighmcculloch · 2022-12-02T18:28:25Z

docs/examples/deployer.mdx

+TODO: the CLI currently doesn't support the install operation. Before running
+the `invoke` below, the contract has to be installed.
+


There's no plan to add an install command to the CLI in this iteration, so we need to remove this todo and come up with a way for them to "install" it.

leighmcculloch · 2022-12-02T18:28:44Z

docs/examples/deployer.mdx

 ```sh
 soroban invoke \
    --wasm target/wasm32-unknown-unknown/release/soroban_deployer_contract.wasm \
    --id 0 \
    --fn deploy \
    --arg 0000000000000000000000000000000000000000000000000000000000000000 \
-    --arg $(xxd -p -c- target/wasm32-unknown-unknown/release/soroban_deployer_test_contract.wasm) \
+    --arg <installed WASM hash> \


We need to tell people how to get this hash. Can you add that?

dmkozh

Updated the phrasing according to comments. I'll come back to the CLI-related part when we have some changes for CLI merged.

dmkozh · 2022-12-02T19:44:13Z

docs/examples/deployer.mdx

-Open the `deployer/deployer/src/src.rs` file to follow along.
+Open the `deployer/deployer/src/lib.rs` file to follow along.
+
+### Contract code installation


Capitalized. I think we need to keep 'code' around though, as we are not installing the contract yet (IMO 'contract deploy' and 'contract install' aren't really distinguishable and both concern 'contracts', while the code is not a contract).

docs/examples/deployer.mdx

dmkozh · 2022-12-02T19:45:47Z

docs/examples/deployer.mdx

+[Below](#tests) is the example of installing the contract code in tests. For the actual
+installation see the general deployment [tutorial](https://soroban.stellar.org/docs/tutorials/deploy-to-futurenet).


dmkozh · 2022-12-02T19:47:15Z

docs/examples/deployer.mdx


 ### Deployer

 The `deployer` contract deploys other contracts. It accepts a salt that will be
-used to derive a contract ID, and a WASM bytes to deploy. It also accepts an
-initialization function and arguments to pass to that function.
+used to derive a contract ID, and a hash-based identifier of the installed WASM


Not sure about removing this... Maybe we should really only hash the code (initially the install entries had more fields), but that would need to happen in the next release. Added an additional comment to make that clear.

dmkozh · 2022-12-02T19:52:26Z

docs/examples/deployer.mdx

-used to derive a contract ID, and a WASM bytes to deploy. It also accepts an
-initialization function and arguments to pass to that function.
+used to derive a contract ID, and a hash-based identifier of the installed WASM
+code to deploy. It also accepts an initialization function and arguments to pass 


dmkozh · 2022-12-02T20:01:17Z

docs/examples/deployer.mdx

-WASM bytes. The WASM bytes will be deployed and the contract ID for that new
-contract is returned. The contract ID is deterministic and is derived from the
-deploying contract and the salt.
+WASM hash. The contract that with the code corresponding to the `wasm_hash` will


Rephrased and added more context regarding properties/benefits of hash-based deployment.

dmkozh · 2022-12-02T20:03:18Z

docs/examples/deployer.mdx

-    let client = DeployerClient::new(&env, &deployer_id);
+    let client = DeployerClient::new(&env, &env.register_contract(None, Deployer));
+
+    // Install the WASM code to be deployed from the deployer contract.


dmkozh · 2022-12-02T20:04:39Z

docs/examples/deployer.mdx

-The deployer contract is registered with the environment.
+Register the deployer contract with the environment:


dmkozh · 2022-12-02T20:05:55Z

docs/examples/deployer.mdx


 ```rust
 env.register_contract(&deployer_id, Deployer);
 ```

+...and created a client for it:


stellar-jenkins · 2022-12-02T20:14:23Z

Preview is available here:
http://soroban-docs-pr230.previews.kube001.services.stellar-ops.com/

This commit should be reverted if the respective CLI PR doesn't get into release.

stellar-jenkins · 2022-12-05T23:59:53Z

Preview is available here:
http://soroban-docs-pr230.previews.kube001.services.stellar-ops.com/

docs/examples/deployer.mdx

stellar-jenkins · 2022-12-06T00:34:21Z

Preview is available here:
http://soroban-docs-pr230.previews.kube001.services.stellar-ops.com/

dmkozh changed the base branch from main to dev December 2, 2022 18:12

Updated the deployer example.

a6a76dc

This is still incomplete: - The examples tag should be bumped - The CLI should be updated, so that there is some way to install the contract WASM

dmkozh force-pushed the deployer_upd branch from 6d27954 to a6a76dc Compare December 2, 2022 18:18

leighmcculloch reviewed Dec 2, 2022

View reviewed changes

!fixup doc update

b814333

dmkozh commented Dec 2, 2022

View reviewed changes

leighmcculloch approved these changes Dec 2, 2022

View reviewed changes

Add example of using the 'install' command of the CLI.

ca75052

This commit should be reverted if the respective CLI PR doesn't get into release.

leighmcculloch reviewed Dec 6, 2022

View reviewed changes

docs/examples/deployer.mdx Outdated Show resolved Hide resolved

!fixup Rephrased install section.

82821f8

leighmcculloch approved these changes Dec 6, 2022

View reviewed changes

leighmcculloch merged commit 702f395 into stellar-deprecated:dev Dec 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated the deployer example. #230

Updated the deployer example. #230

dmkozh commented Dec 1, 2022

stellar-jenkins commented Dec 1, 2022

stellar-jenkins commented Dec 2, 2022

leighmcculloch left a comment

leighmcculloch Dec 2, 2022

dmkozh Dec 2, 2022

leighmcculloch Dec 2, 2022

dmkozh Dec 2, 2022

leighmcculloch Dec 2, 2022

leighmcculloch Dec 2, 2022

dmkozh Dec 2, 2022

leighmcculloch Dec 2, 2022

dmkozh Dec 2, 2022

leighmcculloch Dec 2, 2022

leighmcculloch Dec 2, 2022

dmkozh Dec 2, 2022

leighmcculloch Dec 2, 2022

dmkozh Dec 2, 2022

leighmcculloch Dec 2, 2022

leighmcculloch Dec 2, 2022

dmkozh left a comment

dmkozh Dec 2, 2022

dmkozh Dec 2, 2022

dmkozh Dec 2, 2022

dmkozh Dec 2, 2022

dmkozh Dec 2, 2022

dmkozh Dec 2, 2022

dmkozh Dec 2, 2022

dmkozh Dec 2, 2022

stellar-jenkins commented Dec 2, 2022

stellar-jenkins commented Dec 5, 2022

stellar-jenkins commented Dec 6, 2022

	instances. The installation typically should happen outside of the deployer
	instances. The installation should typically happen outside of the deployer

		[Below](#tests) is the example of installing the contract code in tests. For the actual
		installation see the general deployment [tutorial](https://soroban.stellar.org/docs/tutorials/deploy-to-futurenet).

	used to derive a contract ID, and a hash-based identifier of the installed WASM
	used to derive a contract ID, and a hash of the installed WASM

	...and created a client for it:
	Create a client that will be used to invoke the contract.

		The deployer contract is registered with the environment.
		Register the deployer contract with the environment:

		TODO: the CLI currently doesn't support the install operation. Before running
		the `invoke` below, the contract has to be installed.

Updated the deployer example. #230

Updated the deployer example. #230

Conversation

dmkozh commented Dec 1, 2022

stellar-jenkins commented Dec 1, 2022

stellar-jenkins commented Dec 2, 2022

leighmcculloch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmkozh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stellar-jenkins commented Dec 2, 2022

stellar-jenkins commented Dec 5, 2022

stellar-jenkins commented Dec 6, 2022