passing error from sendTx as response of send raw tx #218
Conversation
Codecov Report
@@ Coverage Diff @@
## master #218 +/- ##
==========================================
+ Coverage 94.46% 94.47% +0.01%
==========================================
Files 70 70
Lines 1030 1032 +2
Branches 193 193
==========================================
+ Hits 973 975 +2
Misses 51 51
Partials 6 6
Continue to review full report at Codecov.
|
|
don't you break the compatibility with Ethereum RPC by changing the response? https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_sendrawtransaction |
| return [false, e.toString()]; | ||
| let err = e.toString(); | ||
| if (err === '[object Object]') { | ||
| err = JSON.stringify(e); |
There was a problem hiding this comment.
Agree with @johannbarbie . We should be smarter and add some error messages to the tx handlers. Also we don't know if that could be a security issue if we return the whole stringified object 🤔
There was a problem hiding this comment.
Yes, I agree with you but I was not sure how to do batter
@johannbarbie Do you think it's better just to send error instead of tx hash in case there is an error?
@pinkiebell The only other way I see it is to find out why vmerror is not thrown as proper exception and fix it there
There was a problem hiding this comment.
Do you think it's better just to send error instead of tx hash in case there is an error?
yes, the user knows the txHash anyways .
What he really cares about is the moment when the tx is included.
i think the way it works in ethereum is 2 steps:
- from
sendRawTransaction()you get a txHash back - with the txHash you use
getTransactionReceipt()to wait for the block to be mined
https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_gettransactionreceipt
i think we should follow the API, and start thinking about receipts, but outside of this bounty.
There was a problem hiding this comment.
yes, the user knows the txHash anyways
ok, fixed that
and agree, it would be great if we can get error by calling getTransactionReceipt
tx ash in case of success or error in case of an error
|
this is not compatible with changes introduced in #241 |
fixes #207
sendRawTransactionnow returns an object{hash, error}witherror: 0in case of success