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

Fix diagnostic event parsing bug #199

Merged
merged 1 commit into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion internal/input/changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ func extractBatch(
xdr.LedgerEntryTypeClaimableBalance,
xdr.LedgerEntryTypeContractData,
xdr.LedgerEntryTypeContractCode,
xdr.LedgerEntryTypeConfigSetting}
xdr.LedgerEntryTypeConfigSetting,
xdr.LedgerEntryTypeExpiration}

changes := map[xdr.LedgerEntryType][]ingest.Change{}
ctx := context.Background()
Expand Down
10 changes: 8 additions & 2 deletions internal/transform/diagnostic_events.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ func TransformDiagnosticEvent(transaction ingest.LedgerTransaction, lhe xdr.Ledg
var transformedDiagnosticEvents []DiagnosticEventOutput

for _, diagnoticEvent := range transactionMeta.SorobanMeta.DiagnosticEvents {
var outputContractId string

outputInSuccessfulContractCall := diagnoticEvent.InSuccessfulContractCall
event := diagnoticEvent.Event
outputExtV := event.Ext.V
contractIdByte, _ := event.ContractId.MarshalBinary()
outputContractId, _ := strkey.Encode(strkey.VersionByteContract, contractIdByte)
outputType := event.Type.String()
outputBodyV := event.Body.V
body, ok := event.Body.GetV0()
Expand All @@ -51,6 +51,12 @@ func TransformDiagnosticEvent(transaction ingest.LedgerTransaction, lhe xdr.Ledg
continue
}

if event.ContractId != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious, in what scenarios would the ContractId be null? Would this be null during the original WASM upload to host?

Copy link
Contributor Author

@chowbao chowbao Oct 3, 2023

Choose a reason for hiding this comment

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

Yes that's one case but I also think it's unoptimized diagnostic events.

Example transaction_hash: 559f7422e90d054d77cbc32a756af55ff9838ca077f75b2e8b8329971b9eb57c
The transaction failed because invokeHostFunctionResourceLimitExceeded but it doesn't give you the contract_id that was used because the resource error is associated with the transaction and not necessarily a problem with the contract being invoked.

Could it have given a contract_id? Yes but it doesn't 🤷‍♀️
Can we find out the contract_id? Probably yes, with joins to transaction_id --> history_operations --> contract_id

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, that makes sense. I don't think we need to always find the contract_id, we can leave that up to the user if they need to trace

contractId := *event.ContractId
contractIdByte, _ := contractId.MarshalBinary()
outputContractId, _ = strkey.Encode(strkey.VersionByteContract, contractIdByte)
}

transformedDiagnosticEvent := DiagnosticEventOutput{
TransactionHash: outputTransactionHash,
LedgerSequence: outputLedgerSequence,
Expand Down
Loading