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

added from and to to Receipt #8756

Merged
merged 1 commit into from
Jun 7, 2018
Merged

added from and to to Receipt #8756

merged 1 commit into from
Jun 7, 2018

Conversation

ab320012
Copy link
Contributor

@ab320012 ab320012 commented May 31, 2018

#6948
Opening a PR to track the issue.

I am having some issues testing this. After following initial set up for macos x I start parity and then call with the method eth_getTransactionReceipt; with any transaction it returns a null result

curl -X POST localhost:8545 -H 'content-type: application/json'
-d '{
"jsonrpc": "2.0",
"method": "eth_getTransactionReceipt",
"params": ["0x60a37c38327d3f3b2cace6ad89546895bf49c1fb6c70a4b23c9027967232ecb8"],
"id": 9
}'
=>
{"jsonrpc":"2.0","result":null,"id":1}

I am thinking its a config issue not sure if anyone can assist.

@Tbaut
Copy link
Contributor

Tbaut commented May 31, 2018

Your node is most probably not synced. You should start parity with the --chain dev flag, to launch the development chain, then make a transaction (you can use Parity UI to send eth to yourself with the pre-configured account) and finally query the transaction.

@ab320012
Copy link
Contributor Author

ab320012 commented Jun 1, 2018

@Tbaut thanks for the explanation.

Running into more issues: installed parity-ui. starting parity in the following way:
./target/release/parity --chain dev --ui-no-validation

then in parity-ui project: npm start

when i route to localhost:3000 i get a loading screen:
screen shot 2018-06-01 at 7 48 28 am

@Tbaut
Copy link
Contributor

Tbaut commented Jun 1, 2018

Never saw that. Could you install it with the binaries?

@5chdn 5chdn added A0-pleasereview 🤓 Pull request needs code review. M6-rpcapi 📣 RPC API. labels Jun 2, 2018
@5chdn 5chdn added this to the 1.12 milestone Jun 2, 2018
@ab320012
Copy link
Contributor Author

ab320012 commented Jun 4, 2018

commenting for gitcoin tracking - the issue is almost done just working out the failing tests

@ab320012 ab320012 force-pushed the master branch 11 times, most recently from fcd624a to 0cf0e0d Compare June 6, 2018 17:06
@vs77bb
Copy link

vs77bb commented Jun 6, 2018

Thanks for the update @ab320012! Snoozed Gitcoin Bot 👍

@ab320012
Copy link
Contributor Author

ab320012 commented Jun 6, 2018

@5chdn @Tbaut all the tests pass i think this is done

@folsen
Copy link
Contributor

folsen commented Jun 7, 2018

@tomusdrw LGTM, this doesn't seem to introduce that second DB read you were talking about, could you please give this a look?

Copy link
Collaborator

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

Yeah it seems that we are doing multiple reads anyway to figure out the transaction location. Looks good to me, but note that the fields are missing again in "the spec":

https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_gettransactionreceipt

@@ -157,6 +157,10 @@ pub struct LocalizedReceipt {
pub log_bloom: Bloom,
/// Transaction outcome.
pub outcome: TransactionOutcome,
/// Receiver address
pub to: Option<H160>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note we now have:

contract_address: Option<_>,
to: Option<_>,

and exactly one of the two will be Some for every transaction.

Just making sure that what's speced.

@folsen
Copy link
Contributor

folsen commented Jun 7, 2018

@tomusdrw Actually, I see what the problem is, the documentation itself is really inconsistent.

There's: https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_gettransactionreceipt
That says it shouldn't be included.
And there's: https://github.com/ethereum/wiki/wiki/JavaScript-API#web3ethgettransactionreceipt

That includes it, but this could be interpreted as the javascript library should be returning the fields on its own (this is obviously not how it works though).

I'm actually not sure what the right thing to do here is.

@ab320012
Copy link
Contributor Author

ab320012 commented Jun 7, 2018

Why not try to update the docs for eth get transaction receipt in ethereum json RPC as it's out of sync with reality

edit:
@folsen I actually just did this

@5chdn 5chdn added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 7, 2018
@5chdn 5chdn merged commit a6d267a into openethereum:master Jun 7, 2018
@Tbaut
Copy link
Contributor

Tbaut commented Jun 8, 2018

Need a PR for the wiki too

dvdplm added a commit that referenced this pull request Jun 11, 2018
* master:
  Fix subcrate test compile (#8862)
  network-devp2p: downgrade logging to debug, add target (#8784)
  Clearing up a comment about the prefix for signing (#8828)
  Disable parallel verification and skip verifiying already imported txs. (#8834)
  devp2p: Move UDP socket handling from Discovery to Host. (#8790)
  Fixed AuthorityRound deadlock on shutdown, closes #8088 (#8803)
  Specify critical release flag per network (#8821)
  Fix `deadlock_detection` feature branch compilation (#8824)
  Use system allocator when profiling memory (#8831)
  added from and to to Receipt (#8756)
ordian added a commit to ordian/parity that referenced this pull request Jun 20, 2018
…rp_sync_on_light_client

* 'master' of https://github.com/paritytech/parity:
  Fix `deadlock_detection` feature branch compilation (openethereum#8824)
  Use system allocator when profiling memory (openethereum#8831)
  added from and to to Receipt (openethereum#8756)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M6-rpcapi 📣 RPC API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants