rpc: adjust sendrawtransaction and submitblock RPC calls
#1216
Conversation
AnnaShaleva
force-pushed
the
neo3/rpc/sendrawtransaction
branch
from
4e49c1e
to
704be2d
Compare
AnnaShaleva
force-pushed
the
neo3/rpc/sendrawtransaction
branch
from
704be2d
to
488e8dd
Compare
Codecov Report
@@ Coverage Diff @@
## master #1216 +/- ##
==========================================
+ Coverage 61.05% 66.89% +5.83%
==========================================
Files 199 199
Lines 16971 16976 +5
==========================================
+ Hits 10362 11356 +994
+ Misses 6055 5011 -1044
- Partials 554 609 +55
Continue to review full report at Codecov.
|
pkg/rpc/client/rpc.go
Outdated
| @@ -385,18 +385,15 @@ func (c *Client) invokeSomething(method string, p request.RawParams, cosigners [ | |||
| // The given hex string needs to be signed with a keypair. | |||
| // When the result of the response object is true, the TX has successfully | |||
| // been broadcasted to the network. | |||
| func (c *Client) SendRawTransaction(rawTX *transaction.Transaction) error { | |||
| func (c *Client) SendRawTransaction(rawTX *transaction.Transaction) (*result.RelayResult, error) { | |||
There was a problem hiding this comment.
Do we really need RelayResult wrapper? I think this API can be simplified to return Uint256 and an error.
There was a problem hiding this comment.
Removed this wrapper from RPC client, but we still need it in RPC server for proper JSON marshalling.
pkg/rpc/client/rpc.go
Outdated
| @@ -453,8 +450,7 @@ func (c *Client) SignAndPushInvocationTx(script []byte, acc *wallet.Account, sys | |||
| return txHash, errors.Wrap(err, "failed to sign tx") | |||
| } | |||
| txHash = tx.Hash() | |||
| err = c.SendRawTransaction(tx) | |||
|
|
|||
| _, err = c.SendRawTransaction(tx) | |||
There was a problem hiding this comment.
So what if it's different from txHash?
There was a problem hiding this comment.
Well, there was a check, but #1216 (comment). So, should we also return an error in this case?
There was a problem hiding this comment.
It shouldn't ever happen, but it can. And if it can, it must be handled in some way. At the moment you're just ignoring this potential mismatch and returning txHash, but what's the value of this txHash for the user if it doesn't match server's hash? And it can happen easily with misconfigured client, when you're using some privnet client for mainnet network, for example.
We can of course just not compute this hash client-side, relying on server's answer. Theoretically it would allow us to be network-agnostic on the client, but I don't think we could do it for all the calls and we require proper network setup for the client anyway for everything to function correctly.
So I think we should add this safety check, just in case.
There was a problem hiding this comment.
Added check, now SignAndPushInvocationTx returns an error containing both hashes in case when hashes mismatch.
AnnaShaleva
force-pushed
the
neo3/rpc/sendrawtransaction
branch
from
488e8dd
to
3ca777a
Compare
It should return tx has instead of boolean.
It should return block hash instead of boolean.
AnnaShaleva
force-pushed
the
neo3/rpc/sendrawtransaction
branch
from
3ca777a
to
889a5d7
Compare
These RPC calls should return hashes instead of boolean values.