If contract instantiates sub-contracts, cargo contract instantiate returns wrong contract account id

[cmichi]

Bug

If a contract instantiates sub-contracts, cargo contract instantiate displays the wrong contract account id in its output. It displays the id of the sub-contract.

I'm referring to this output:

cargo contract instantiate …
 Dry-running new (skip with --skip-dry-run)
    Success! Gas required estimated at …
Confirm transaction details: (skip with --skip-confirm)
 Constructor new
        Args …
   Gas limit …
Submit? (Y/n): Y
   Code hash …
    Contract 5HjX3tcg1vccCnxdvPBE2S5mfXjC41dQ6D2wWWohvUdbneTn
      Events
      …

How to replicate

Our delegator example instantiates three sub-contracts in its constructor and shows the bug.

cd ink/examples/delegator/
/bin/bash ./build-all.sh

cargo contract upload --manifest-path=accumulator/Cargo.toml --suri //Alice
cargo contract upload --manifest-path=adder/Cargo.toml --suri //Alice
cargo contract upload --manifest-path=subber/Cargo.toml --suri //Alice

ACCUMULATOR_HASH=$(cat target/ink/accumulator/*.contract | jq -r .source.hash)
ADDER_HASH=$(cat target/ink/adder/*.contract | jq -r .source.hash)
SUBBER_HASH=$(cat target/ink/subber/*.contract | jq -r .source.hash)

cargo contract instantiate --manifest-path=Cargo.toml --suri //Alice --args 0 0 $ACCUMULATOR_HASH $ADDER_HASH $SUBBER_HASH

# Copy the `Contract` address from the above output 
export CONTRACT=…

cargo contract call -m switch --dry-run --suri //Alice  --contract $CONTRACT

The method switch exists only on the delegator/lib.rs contract, but we call the first instantiated sub-contract here (due to the bug) and it doesn't have a method switch. The command thus fails:

$ cargo contract call -m switch --dry-run  --contract $CONTRACT --suri //Alice
          Result ModuleError: Contracts::ContractTrapped: ["Contract trapped during execution."]
          …

Where to start looking

instantiate.rs:

let instantiated = result
    .find_first::<api::contracts::events::Instantiated>()?
    .ok_or_else(|| anyhow!("Failed to find Instantiated event"))?;

I think this should be find_last instead, as the Instantiated event from the first sub-contract is otherwise taken.

What else to think about

Above the code linked right above there is the same find_first for CodeStored:

// The CodeStored event is only raised if the contract has not already been uploaded.
let code_hash = result
    .find_first::<api::contracts::events::CodeStored>()?
    .map(|code_stored| code_stored.code_hash);

I'm not sure if it should be find_last there as well. For this to be faulty the contract would have to be able to upload another contract and I don't think that's possible right now.

[jjmiranda]

That's true, I spend a lot of hours to note this bug 👎
Revised the log, I notice about this error (reverse order):
system:ExtrinsicSuccess
[{"weight":{"refTime":"6,812,732,864","proofSize":"26,888"},"class":"Normal","paysFee":"Yes"}]
transactionPayment:TransactionFeePaid
["5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY","98,993,850","0"]
balances:Reserved
["5GCxJC3o2foWNpSkc5MUf1SiGXWqaTFGpzyFfaAoSzfokf8M","100,340,000,000"]
balances:Transfer
["5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY","5GCxJC3o2foWNpSkc5MUf1SiGXWqaTFGpzyFfaAoSzfokf8M","100,340,000,000"]
balances:Reserved
["5CjxfvE5faUodD83yEugiRrbezwptpGvRXJeDBKsjEp7YWS9","100,020,000,000"]
balances:Transfer
["5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY","5CjxfvE5faUodD83yEugiRrbezwptpGvRXJeDBKsjEp7YWS9","100,020,000,000"]
balances:Reserved
["5EYCponRWoFwXLj7a8CgmupCSxsQkR7krzptBY4CkBR39Pou","100,020,000,000"]
balances:Transfer
["5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY","5EYCponRWoFwXLj7a8CgmupCSxsQkR7krzptBY4CkBR39Pou","100,020,000,000"]
contracts:Instantiated
["5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY","5GCxJC3o2foWNpSkc5MUf1SiGXWqaTFGpzyFfaAoSzfokf8M"]
contracts:Instantiated
["5GCxJC3o2foWNpSkc5MUf1SiGXWqaTFGpzyFfaAoSzfokf8M","5CjxfvE5faUodD83yEugiRrbezwptpGvRXJeDBKsjEp7YWS9"]
balances:Reserved
["5CjxfvE5faUodD83yEugiRrbezwptpGvRXJeDBKsjEp7YWS9","100,605,000,000"]
balances:Transfer
["5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY","5CjxfvE5faUodD83yEugiRrbezwptpGvRXJeDBKsjEp7YWS9","100,605,000,000"]
balances:Endowed
["5CjxfvE5faUodD83yEugiRrbezwptpGvRXJeDBKsjEp7YWS9","100,605,000,000"]
system:NewAccount
["5CjxfvE5faUodD83yEugiRrbezwptpGvRXJeDBKsjEp7YWS9"]
contracts:Instantiated
["5GCxJC3o2foWNpSkc5MUf1SiGXWqaTFGpzyFfaAoSzfokf8M","5EYCponRWoFwXLj7a8CgmupCSxsQkR7krzptBY4CkBR39Pou"]
balances:Reserved
["5EYCponRWoFwXLj7a8CgmupCSxsQkR7krzptBY4CkBR39Pou","100,605,000,000"]
balances:Transfer
["5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY","5EYCponRWoFwXLj7a8CgmupCSxsQkR7krzptBY4CkBR39Pou","100,605,000,000"]
balances:Endowed
["5EYCponRWoFwXLj7a8CgmupCSxsQkR7krzptBY4CkBR39Pou","100,605,000,000"]
system:NewAccount
["5EYCponRWoFwXLj7a8CgmupCSxsQkR7krzptBY4CkBR39Pou"]
balances:Reserved
["5GCxJC3o2foWNpSkc5MUf1SiGXWqaTFGpzyFfaAoSzfokf8M","100,605,000,000"]
balances:Transfer
["5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY","5GCxJC3o2foWNpSkc5MUf1SiGXWqaTFGpzyFfaAoSzfokf8M","100,605,000,000"]
balances:Endowed
["5GCxJC3o2foWNpSkc5MUf1SiGXWqaTFGpzyFfaAoSzfokf8M","100,605,000,000"]
system:NewAccount
["5GCxJC3o2foWNpSkc5MUf1SiGXWqaTFGpzyFfaAoSzfokf8M"]
balances:Withdraw
["5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY","98,993,850"]

cargo contract instantiate shows this accountID:
5EYCponRWoFwXLj7a8CgmupCSxsQkR7krzptBY4CkBR39Pou

This is the second system:NewAccount and first contracts:Instantiated.

The correct accountID for cargo contract instantiate would be:
5GCxJC3o2foWNpSkc5MUf1SiGXWqaTFGpzyFfaAoSzfokf8M
The first system:NewAccount