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 note on differences from standard CosmWasm Sub-message Behavior #165

Open
wants to merge 1 commit into
base: gitbook
Choose a base branch
from

Conversation

luca992
Copy link
Contributor

@luca992 luca992 commented Oct 19, 2022

Sub-message behavior is different on secret network than standard CosmWasm and I think it should be noted

Sub-message behavior is different on secret network than standard CosmWasm and I think it should be noted
Comment on lines +128 to +131
Note differences from standard CosmWasm:
* Obtaining contract data instantiated w/ sub-messages:
* `msg.result.events` from `msg: Reply` passed into `#entry_point pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> StdResult<Response>` should be used to obtain data, such as `contract_address`, after instantiating a contract. Standard CosmWasm will return `MsgInstantiateContractResponse` data as protobuf binary inside `msg.result.data`. However `msg.result.data` currently returns no binary data on Secret Network v1.4. Thus, utilies such as [`parse_instantiate_response_data`](https://github.com/scrtlabs/cw-plus/blob/14a9db7e8b93039fa856cbe126ffdb230a00734a/packages/utils/src/parse_reply.rs#L106) from `cw-utils` will not work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey, I'm not sure I understand the difference any chance you can comment with an example of how the data is serialized on other cw 1 chain compared to SCRT serialization?

Copy link
Contributor Author

@luca992 luca992 Oct 23, 2022

Choose a reason for hiding this comment

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

The difference is that cw 1 chains have
msg.result.data = **MsgInstantiateContractResponse protobuf binary**

for example terraswap decodes that data here:
https://github.com/terraswap/terraswap/blob/48e399eaa2e89cca4da8c7b20c9f5512a47ec36d/contracts/terraswap_factory/src/contract.rs#L231

But on secret 1.4.0: msg.result.data just returns null

I'm guessing it's a bug, based on this comment:

// note: even if contractAddr == nil then contractAddr.String() is ok // \o/🤷🤷‍♂️🤷‍♀️🤦🤦‍♂️🤦‍♀️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

INFO  [enclave_contract_engine::wasm::engine] Invoking reply() in wasm
INFO  [enclave_contract_engine::wasm::contract] debug_print: "AAAAAAAAA!"
INFO  [enclave_contract_engine::wasm::contract] debug_print: "msg.result.unwrap().data: !"
INFO  [enclave_contract_engine::wasm::contract] debug_print: "msg.result.unwrap().events: [Event { ty: \"instantiate\", attributes: [Attribute { key: \"code_id\", value: \"3\", encrypted: true }, Attribute { key: \"contract_address\", value: \"secret1sh36qn08g4cqg685cfzmyxqv2952q6r8vqktuh\", encrypted: true }] }, Event { ty: \"wasm\", attributes: [Attribute { key: \"contract_address\", value: \"secret1sh36qn08g4cqg685cfzmyxqv2952q6r8vqktuh\", encrypted: true }] }]!"

image

Copy link
Contributor Author

@luca992 luca992 Oct 23, 2022

Choose a reason for hiding this comment

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

As you can see no data is returned for msg.result.data
But, on other chains they get:
msg.result.data = **MsgInstantiateContractResponse protobuf binary**

@ertemann
Copy link
Collaborator

@liorbond can this be merged or do we need to explain it differently?

@luca992
Copy link
Contributor Author

luca992 commented Feb 20, 2023

Maybe it should just be an issue in the secret network repo @ertemann

@luca992
Copy link
Contributor Author

luca992 commented Feb 20, 2023

Unless the behavior is intended

@liorbond
Copy link
Collaborator

@ertemann I agree with @luca992 I think it should be an issue I'm almost sure it wasn't something we intend to do. We will need to get into it in order to be sure.

@ertemann
Copy link
Collaborator

ertemann commented Jul 3, 2023

@liorbond @luca992 was this fixed or reported as an issue in the SN codebase?

@SecretSaturn
Copy link
Contributor

Is this still relevant?

@luca992
Copy link
Contributor Author

luca992 commented Apr 9, 2024

I made an issue here. I don't think anything was done about this.

scrtlabs/SecretNetwork#1323

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.

4 participants