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

Save contract info before calling instantiate entrypoint #2044

Open
chipshort opened this issue Nov 27, 2024 · 8 comments
Open

Save contract info before calling instantiate entrypoint #2044

chipshort opened this issue Nov 27, 2024 · 8 comments

Comments

@chipshort
Copy link
Collaborator

Current behaviour

The instantiate entrypoint is called before the contract info is stored.
This results in an error occuring if the contract tries to query that info inside the instantiate entrypoint.

Expected behaviour

We should probably call the entrypoint after saving the info. That should make the data available to query inside the instantiate entrypoint. To me, it doesn't look like there is any specific reason for the current order and it is confusing and a somewhat inconsistent1.
We should just make sure that it gets reverted if the instantiate entrypoint errors. I think that should be the case automatically when returning an error, but a test for that would be good.

Footnotes

  1. See https://github.com/CosmWasm/cw-multi-test/issues/226 and https://discord.com/channels/737637324434833438/737643344712171600/1303704162298630186

@kulikthebird
Copy link
Contributor

There is one scenario I'd consider before implementing this. What would happen if the contract query itself from the instantiate method before initiating the state. Other than that it seems that it's safe to add the contract address to the instantiate call args.

@DariuszDepta
Copy link
Member

Contract address is already here: env.contract.address passed as an argument to instantiate entry-point.
When the contract tries to query itself inside the instantiate entry-point the same error is reported like for contract code info:

{
  "data": {
    "value": "Generic error: Querier system error: No such contract: wasm1nc5tatafv6eyq7llkr2gv50ff9e22mnf70qgjlv737ktmt4eswrqr5j2ht"
  }
}

I guess it works totally fine, and in my opinion this functionality should stay as it is. Instantiate entry-point may always return an error, so there is no reason to use its address to interact in any ways with the blockchain, also querying the contract_code_info should fail as it is now. We should just align the MT behaviour. As I understand the original request, the problem is, that MT behaviour differs from wasmd, and not that wasmd should provide contract code info inside the instantiate function (for some strange reason). So I suggest to do changes in MT not in wasmd.

@DariuszDepta
Copy link
Member

Additionally, in my opinion, during the instantiation, the code_id is useless for the contract. One can store the same code multiple times and instantiate logically the same smart contract based on different code_id. I can not imagine a use case where checking the value of code_id inside instantiate may be useful in any ways. But I may be wrong...

@Kayanski
Copy link

Kayanski commented Dec 4, 2024

Additionally, in my opinion, during the instantiation, the code_id is useless for the contract. One can store the same code multiple times and instantiate logically the same smart contract based on different code_id. I can not imagine a use case where checking the value of code_id inside instantiate may be useful in any ways. But I may be wrong...

Thank you for contacting me off-github to talk about that issue. We indeed have a use-case, where we need this feature.
To solve that problem, we have thought of 2 solutions :

  • The solution you propose here to register the contract in state before (to be able to query it). It definitely makes sens.
  • Make code_id available inside the rust Contract variable, so you could call env.contract.code_id. (breaking)

We are fine with which-ever, but we would prefer a non-breaking wasmd patch to be able to have that quickly out there.

Use Case

As discussed, here is my use-case (actual Abstract use-case ).

Inside the instantiation of our account contract, we need the query the creator of the code_id (bascially the ABstract owner) to be able to get the address of our contracts that were deployed by this creator using instantiate2. This creator can be queried from the code id of the contract. SO we indeed need the code_id of the contract within the instantiation of the contract.

This was introduced because we have storage constraints on the chain we target that don't allow us to store too much address per abstract account

Thank you !

@DariuszDepta
Copy link
Member

Did some tests on the node using newest wasmd. Results:
Calling:

let contract_addr = env.contract.address;
deps.querier.query_wasm_contract_info(contract_addr);

inside instantiate entry-point returns an error and this should be fixed in wasmd.
Calling:

let code_id = 1u64;
deps.querier.query_wasm_code_info(code_id);

inside instantiate entry-point works in wasmd, but one has to know the code_id. The code_id for a contract may only be retrieved using query_wasm_contract_info but this does not work in wasmd. Calling query_wasm_smart inside instantiate function for the instantiated contract should return an error as it does now, because the contract is not yet fully instantiated.
Feel free to comment on this @chipshort @kulikthebird

source code

@chipshort
Copy link
Collaborator Author

Thanks for looking into that, Darek.
I agree with both you and Tomek that querying the contract inside instantiate should fail. That's a really great point, in fact, because with my original proposal it would not fail. I'm not sure what the best way to remedy this would be. Maybe add a boolean like finished_instantiation to the ContractInfo? (only on wasmd side, the contract doesn't need to know about it)

@Kayanski
Copy link

Kayanski commented Dec 6, 2024

Thanks for those experimentations.
To solve that issue, let's take the example of a contract execution. During execution, you are not able to query variables on the same smart contract that were just modified.
During instantiation, this would be the same thing, you would be able to query the self contract but with no storage instantiated, so I don't really see an issue there no ? It's just a contract with no storage that you are trying to query there, which should error because there is no data there.

TL;DR: I think querying the contract inside instantiate doesn't have to fail

@chipshort
Copy link
Collaborator Author

chipshort commented Dec 10, 2024

Yes, that's true, but I think it feels a bit weird that you would be able to query the contract before it is instantiated. Technically there cannot come much "harm" from that, but seems a bit inconsistent.
Also, it would not necessarily fail. If the contract handles storage errors or uses default values, it could return unexpected values. But then again, you are explicitly querying that before it is stored, so maybe that's acceptable.

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

No branches or pull requests

4 participants