Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Light node response to eth_getLogs missing transactionHash #9929

Closed
epheph opened this issue Nov 16, 2018 · 12 comments
Closed

Light node response to eth_getLogs missing transactionHash #9929

epheph opened this issue Nov 16, 2018 · 12 comments
Assignees
Labels
F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. M4-core ⛓ Core client code / Rust.
Milestone

Comments

@epheph
Copy link

epheph commented Nov 16, 2018

Before filing a new issue, please provide the following information.

  • Parity Ethereum version: v2.2.1
  • Operating system: Linux
  • Installation: Official docker image
  • Fully synchronized: What's that even mean for a warping light node? 😜
  • Network: ethereum
  • Restarted: yes

Issuing a getLogs request to a parity node that is in --light:

$ curl -s -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_getLogs","params":[ {"fromBlock":"0x5a6da7","toBlock":"0x5a6da7","address":"0x75228dce4d82566d93068a8d5d49435216551599"} ],"id":74}' localhost:8545 | jq .result[0]

{
  "address": "0x75228dce4d82566d93068a8d5d49435216551599",
  "blockHash": "0x4ff6030cce082be8f5a56d7f7bbe5e32e8d98295f31c8e070285fbdd022a06a7",
  "blockNumber": "0x5a6da7",
  "data": "0x000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
  "logIndex": "0x9",
  "removed": false,
  "topics": [
    "0x299eaafd0d27519eda3fe7195b73e5269e442b3d80928f19afa32b6db2f352b6",
    "0x0000000000000000000000000000000000000000000000000000000000000000",
    "0x000000000000000000000000e991247b78f937d7b69cfc00f1a487a293557677"
  ],
  "transactionHash": null,
  "transactionIndex": "0x2a",
  "transactionLogIndex": "0x0",
  "type": "mined"
}

The data is correct and complete, but

  "transactionHash": null,

That response, from a parity full node or geth node (light or full) is:

$  curl -s -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_getLogs","params":[ {"fromBlock":"0x5a6da7","toBlock":"0x5a6da7","address":"0x75228dce4d82566d93068a8d5d49435216551599"} ],"id":74}' my-geth-server | jq .result[0]
{
  "address": "0x75228dce4d82566d93068a8d5d49435216551599",
  "topics": [
    "0x299eaafd0d27519eda3fe7195b73e5269e442b3d80928f19afa32b6db2f352b6",
    "0x0000000000000000000000000000000000000000000000000000000000000000",
    "0x000000000000000000000000e991247b78f937d7b69cfc00f1a487a293557677"
  ],
  "data": "0x000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
  "blockNumber": "0x5a6da7",
  "transactionHash": "0x44c09f8eeff886723b79890e14743192a8c8d8a8eac158ed17600c94e502cce8",
  "transactionIndex": "0x2a",
  "blockHash": "0x4ff6030cce082be8f5a56d7f7bbe5e32e8d98295f31c8e070285fbdd022a06a7",
  "logIndex": "0x9",
  "removed": false
}
@jam10o-new jam10o-new added F2-bug 🐞 The client fails to follow expected behavior. M4-core ⛓ Core client code / Rust. labels Nov 16, 2018
@jam10o-new jam10o-new added this to the 2.3 milestone Nov 16, 2018
@jam10o-new
Copy link
Contributor

@epheph is this replicable with other transactions?

@cheme
Copy link
Contributor

cheme commented Nov 16, 2018

@joshua-mir can confirm from reading the code :
https://github.com/paritytech/parity-ethereum/blob/0f90696528e11e3e697a808e2d7983b9733d02bc/rpc/src/v1/helpers/light_fetch.rs#L356-L357
So it is always true, my first guess would be to say that it is a limitation from the pip/les query spec.

@jam10o-new
Copy link
Contributor

So... Looks like something that is expected behavior?

@jam10o-new jam10o-new added F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. and removed F2-bug 🐞 The client fails to follow expected behavior. labels Nov 16, 2018
@cheme
Copy link
Contributor

cheme commented Nov 16, 2018

It is running 'getReceipts' (https://wiki.parity.io/Light-Ethereum-Subprotocol-(LES)) to get the log entries but there does not seem to be an efficient way to get the transaction hash.
To get the actual value you probably need to call 'eth_getTransactionByBlockNumberAndIndex' rpc equivalent which mean a LES query over the block (when called from light client this query a block body so it is really not cheap).
So it seems like 'expected' in some way.

@epheph
Copy link
Author

epheph commented Nov 16, 2018

Geth light-node will return transactionHash with a getLogs request. I wonder how they do it. If they eth_getTransactionByBlockNumberAndIndex for every single log that comes back, it seems like it would take way too long when you query hundreds of logs at a time.

@cheme
Copy link
Contributor

cheme commented Nov 16, 2018

Looks like they query the block.
https://github.com/ethereum/go-ethereum/blob/f55c26ae6d0f25b4aa6b1bd103720067d1fec9fe/light/odr_util.go#L139
which mean they got to download the block so two queries (one being the full block data).
So it certainly is doable in parity to, but is it best to have an always slow/costy getLog, maybe making two separate rpc can make sense.

@cheme
Copy link
Contributor

cheme commented Nov 16, 2018

In fact it could be possible to make it optional (like a light client specific config).

@epheph
Copy link
Author

epheph commented Nov 16, 2018

I agree that geth and parity should return the same data for the requests, maybe a different RPC endpoint if it will differ. The caller doesn't even know if they are talking to geth/parity and they don't know if they are talking to a light/full node

@pgebheim
Copy link

Agreed @epheph -- This is a point that all the clients need to agree on. The behavior of all of these basic RPC requests simply has to be compatible.

Since a client doesn't know what it's speaking to it makes sense it would have to do the more costly call by default, to keep all RPC responses in parity across all nodes.

Perhaps there should be some spec for an eth client to turn on light client optimizations as well as a standard way of detecting whether it's speaking to a light client.

@Tbaut
Copy link
Contributor

Tbaut commented Nov 16, 2018

This is a point that all the clients need to agree on.
Since a client doesn't know what it's speaking it

Totally agree but the sad reality is that the protocols are different, geth does not support pip and a parity light client != geth light client.
So light clients do know who they are speaking to, until things are unified.

it makes sense it would have to do the more costly call by default, to keep all RPC responses in parity across all nodes.

I'd instinctively think the opposite in terms of cost. Cheap by default, costly if you know what you're doing.

@epheph
Copy link
Author

epheph commented Nov 17, 2018

The JSON-RPC documentation doesn't indicate it will only return some information if it is architecturally convenient. If there is going to be a parity-specific implementation that runs faster but returns less info, it seems like it should be in the parity_ module namespace.

If the field not being returned was something like logIndex I could see the argument a little better, but transactionHash is very impactful given that it is a transaction's most unique identifier and is commonly used for linking to sites like etherscan.

@cheme cheme self-assigned this Nov 18, 2018
@Tbaut
Copy link
Contributor

Tbaut commented Nov 18, 2018

Agreed, since it's a standard eth_ call, you get a point :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

No branches or pull requests

5 participants