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

eth/filters: send rpctransactions in pending-subscription #26126

Merged
merged 2 commits into from
Nov 14, 2022

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Nov 7, 2022

This PR is an alternative to #26034. This PR keeps the internal objects as *types.Transaction, and mutates it into rpc.Transaction right before it's sent on the wire.

At least I think it does, I haven't fully verified it (yet).

@MariusVanDerWijden MariusVanDerWijden changed the title eh/filters: send rpctransactions in pending-subscription eth/filters: send rpctransactions in pending-subscription Nov 7, 2022
@s1na
Copy link
Contributor

s1na commented Nov 9, 2022

Example response:

{
  blockHash: null,
  blockNumber: null,
  from: '0x21a31ee1afc51d94c2efccaa2092ad1028285549',
  gas: '0x32918',
  gasPrice: '0x202170e400',
  maxFeePerGas: '0x202170e400',
  maxPriorityFeePerGas: '0x77359400',
  hash: '0xde8f7e26fd9dc0157c8dfb8d6742a50ee1d13077f7eb675a8b9de42374ad4e6d',
  input: '0xa9059cbb000000000000000000000000a055bdc23d5f8fa59214b18a4c892d2be87d2cc200000000000000000000000000000000000000000000000
0000000001b774000',
  nonce: '0x487572',
  to: '0xdac17f958d2ee523a2206206994597c13d831ec7',
  transactionIndex: null,
  value: '0x0',
  type: '0x2',
  accessList: [],
  chainId: '0x1',
  v: '0x1',
  r: '0x93a2c41a452a8ad03c7e31e12608650205f7b2ab8f73bef174fdb28a046ccaa8',
  s: '0x74c0b4b811807c1cb066e7082dcc9341acbeff68fcd2653c9e21e82c45e1720c'
}

Only thing is blockHash, blockNumber and transactionIndex will always be null for pending txes. But adding omitempty rules to RPCTransaction type will also change the result of getTransactionByHash for pending txes, so I guess it's best to keep as is.

Copy link
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

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

LGTM

@s1na
Copy link
Contributor

s1na commented Nov 9, 2022

Ah gethclient's method might need to change.

Update: seems to work. I think because the JSON encoding of a rpc transaction is unmarshalable to types.Transaction.

@holiman
Copy link
Contributor Author

holiman commented Nov 9, 2022

Update: seems to work. I think because the JSON encoding of a rpc transaction is unmarshalable to types.Transaction.

Could you clarify this a bit? I don't understand if there's anything we need to address here

@s1na
Copy link
Contributor

s1na commented Nov 14, 2022

To clarify, IMO no change needed for gethclient. PR looks good to me!

@holiman holiman added this to the 1.11.0 milestone Nov 14, 2022
@holiman holiman merged commit 8c5ce11 into ethereum:master Nov 14, 2022
shekhirin pushed a commit to shekhirin/go-ethereum that referenced this pull request Jun 6, 2023
…6126)

This PR changes the pending tx subscription to return RPCTransaction types instead of normal Transaction objects. This will fix the inconsistencies with other tx returning API methods (i.e. getTransactionByHash), and also fill in the sender value for the tx.

co-authored by @s1na
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return sender (and chainId) on 'eth_subscribe', ['newPendingTransactions', true]
2 participants