Decode diagnostic events in logs

[leighmcculloch]

The CLI isn't decoding diagnostic events in its logs which makes it hard to parse with the human eye. There are advantages to having the diagnostic events in XDR and we should probably keep it in the logs in XDR but also output the JSON representation.

Discussed at:

• https://discord.com/channels/897514728459468821/1260700355335815379/1260706197762347089

Today this is what is seen:

soroban contract invoke --id CAN6I4YMMDEKWQGMFUDLDTMJ6VAAXTN7KJB2EUTI3MVQLUGYETXKR2OH  --source alice --network testnet -- decrement
2024-07-10T20:43:37.376246Z ERROR stellar_rpc_client::log::diagnostic_events: 0: "AAAAAAAAAAAAAAAAAAAAAgAAAAAAAAADAAAADwAAAAdmbl9jYWxsAAAAAA0AAAAgG+RzDGDIq0DMLQaxzYn1QAvNv1JDolJo2ysF0Ngk7qgAAAAPAAAACWRlY3JlbWVudAAAAAAAAAE="

Ideally with this change this is what the output looks like:

soroban contract invoke --id CAN6I4YMMDEKWQGMFUDLDTMJ6VAAXTN7KJB2EUTI3MVQLUGYETXKR2OH  --source alice --network testnet -- decrement
2024-07-10T20:43:37.376246Z ERROR stellar_rpc_client::log::diagnostic_events: 0: "AAAAAAAAAAAAAAAAAAAAAgAAAAAAAAADAAAADwAAAAdmbl9jYWxsAAAAAA0AAAAgG+RzDGDIq0DMLQaxzYn1QAvNv1JDolJo2ysF0Ngk7qgAAAAPAAAACWRlY3JlbWVudAAAAAAAAAE=" {"in_successful_contract_call":false,"event":{"ext":"v0","contract_id":null,"type_":"diagnostic","body":{"v0":{"topics":[{"symbol":"fn_call"},{"bytes":"1be4730c60c8ab40cc2d06b1cd89f5400bcdbf5243a25268db2b05d0d824eea8"},{"symbol":"decrement"}],"data":"void"}}}}

[BlaineHeffron]

Looks like this needs to be done in the rpc client to address the discord discussion: https://github.com/stellar/rs-stellar-rpc-client/blob/97b12086ecf53ec928514e1472cc20cf8c58e694/src/log/diagnostic_events.rs#L4

Although the same change could also be done here:

stellar-cli/cmd/soroban-cli/src/log/diagnostic_event.rs

Line 4 in 0449dfd

tracing::trace!("{i}: {event:#?}");

I'm not sure what conditions need to occur to get diagnostic events on the CLI end, in my testing when I trigger an error via invoke I only see events from the rpc client.

[leighmcculloch]

The cli, not the rpc client, should be logging event details. The cli should be responsible for communicating to the user and the UI. If the rpc client is currently doing so that's probably a hold over to when they were in the same repo and the boundaries and responsibilities of both were blurred and not maintained.

[BlaineHeffron]

Makes sense, I changed the PR to remove the logging module and the logging statement in the simulate_and_assemble_transaction fn. It will now return both the optional AssembledTransaction and the SimulateTransactionResult. The change requires that the CLI will then have to throw an error if AssembledTransaction is none, in which case it will also be able to log the DiagnosticEvents if there are any in the SimulateTransactionResult.

The follow up PR in the CLI can then add a utility function that encapsulates the simulate_and_assemble_transaction call, performs the check on the result to throw the error and log events if applicable, and return the AssembledTransaction. This can then be used wherever simulate_and_assemble_transaction is called. The more detailed logging logic containing the JSON decode will also be in that PR.

[BlaineHeffron]

I just opened a draft PR to resolve this issue which I can open once the RPC changes are merged. #1499

[leighmcculloch]

The cli, not the rpc client, should be logging event details

One thing I wanted to clarify is that while the cli should be in control of logging things like events, there are some things that are within the scope of the rpc client to log and out-of-scope of the cli, such as raw messages sent and received, and so as part of this change we don't need to rip all logging out of the rpc client.

I don't think anything significant needs changing in the PRs, because the only logging I see being removed from the rpc client is events and results, but just wanted to call that out because I realized I didn't communicate as clearly about it as I could have.