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

feat: add returndata to tx receipts #542

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
4 changes: 4 additions & 0 deletions src/schemas/receipt.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,7 @@ ReceiptInfo:
title: blob gas price
description: The actual value per gas deducted from the sender's account for blob gas. Only specified for blob transactions as defined by EIP-4844.
$ref: '#/components/schemas/uint'
returnData:
title: returndata
description: The returndata from the call. This is an optional field that the client is not required to return if the returndata is not available. If it is not available, it should return `nil`.
Copy link

Choose a reason for hiding this comment

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

the return data is usually not stored because it can retrieved by re-executing the tx again, hence it's only part of the trace responses.

if this is an option, then it's ambiguous what the client impl should do, always re-execute or always return null

but having a standard call for obtaining the returndata would be very useful, so maybe this should be a standalone function instead?

Copy link
Author

Choose a reason for hiding this comment

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

i left it optional exactly for this reason, clients probably do not have the returndata stored, so in the short-term they can keep doing exactly what they have been doing. it is preferred by consumers that the returndata is there, but if it is nil then they can fall back to workaround (as they currently do).

Copy link
Author

Choose a reason for hiding this comment

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

a standardized eth_getReturnData would also work for this purpose, although it seems inefficient since in the (very common) case that you want to inspect the returndata, you need to make two calls instead of just one.

Copy link
Author

Choose a reason for hiding this comment

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

just want to include some points offline - asking for returndata in getTxReceipt may increase latency and/or pricing for infra providers

Copy link
Author

Choose a reason for hiding this comment

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

you need to make two calls instead of just one.

by the way, having two calls is pretty problematic because there are race conditions when you have to call the node twice (there could be a reorg or the node provider could switch clients and the second call returns null or a different value). the ideal state of affairs is to get the result back in one call.

Copy link
Author

@charles-cooper charles-cooper May 23, 2024

Choose a reason for hiding this comment

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

another issue in practice is that RPC providers fail all the time, so a very common case is that you get the result of eth_getTransactionReceipt but then the node fails, and the following call to grab the trace (or alternatives like the above proposed eth_getReturnData) fails. it's not a showstopper, you can do a second retry loop or failover to another RPC. but it's more ideal -- just simpler and less prone to failure -- to get the returndata back in a single call.

$ref: '#/components/schemas/bytes'
Loading