fix: ensure that the sim events are logged; improve format of main logs

[willemneal]

Before, the simulation logs were not logged if there was a failure and looked like:

2023-08-29T18:50:32.285005Z DEBUG soroban_cli::commands::contract::invoke: events=[DiagnosticEvent { in_successful_contract_call: true, event: ContractEvent { ext: V0, contract_id: Some(Hash(6ccda1006dc614a675f61b8f8ecbb2397152b1ee5495795e46d6c654b1e6c271)), type_: Contract, body: V0(ContractEventV0 { topics: ScVec(VecM([Symbol(ScSymbol(StringM(hello))), Symbol(ScSymbol(StringM()))])), data: Symbol(ScSymbol(StringM(world))) }) } }]

After, the simulation logs are always logged and host events and diagnostic events are separated. Also the logs are formatted better.

Now:

2023-08-29T18:58:55.928391Z  INFO soroban_cli::log::diagnostic_event: 0: DiagnosticEvent {
    in_successful_contract_call: true,
    event: ContractEvent {
        ext: V0,
        contract_id: Some(
            Hash(6ccda1006dc614a675f61b8f8ecbb2397152b1ee5495795e46d6c654b1e6c271),
        ),
        type_: Contract,
        body: V0(
            ContractEventV0 {
                topics: ScVec(
                    VecM(
                        [
                            Symbol(
                                ScSymbol(
                                    StringM(hello),
                                ),
                            ),
                            Symbol(
                                ScSymbol(
                                    StringM(),
                                ),
                            ),
                        ],
                    ),
                ),
                data: Symbol(
                    ScSymbol(
                        StringM(world),
                    ),
                ),
            },
        ),
    },
}

Unfortunately as seen here the contract id is still a hash as this is just the debug output from the generated XDR.

[tsachiherman]

@willemneal the PR looks great; could you please run it and add the "before" and "after" to the PR description ?

[willemneal]

@tsachiherman What do you mean by run?

[tsachiherman]

@tsachiherman What do you mean by run?

what is the difference shown in the logs before and after this change ?

[sreuland]

looks good, should help out a bunch on e2e debugging of #855, thx!

[2opremio]

It seems this PR only improves the logging of events for transaction submission.

My comment at #855 (comment) was about preflight.

After #878 the simulateTransaction method will return diagnostic events when there is error and thus the code at https://github.com/stellar/soroban-tools/blob/main/cmd/soroban-cli/src/rpc/mod.rs#L595 should be updated to include the diagnostic events.

@willemneal Can you update that too?

[tsachiherman]

It seems this PR only improves the logging of events when submitting a transaction.

My comment at #855 (comment) was about preflight.

After #878 the simulateTransaction method will return diagnostic events when there is error and thus the code at https://github.com/stellar/soroban-tools/blob/main/cmd/soroban-cli/src/rpc/mod.rs#L595 should be updated to include the diagnostic events.

Can you update that too?

I think it wouldn't be possible to test it properly without a working&updated rpc... And I do agree with your point.

[2opremio]

Thanks @willemneal . I was thinking it may have been better to incorporate the events in the error itself (so that the caller controls when to print them), but it’s probably fine this way.

[willemneal]

Yeah, there is already logging in the file, but I agree a better logging at the api layer would be better.