substrate icon indicating copy to clipboard operation
substrate copied to clipboard

rpc: Implement `transaction` RPC API

Open lexnv opened this issue 1 year ago • 1 comments

This PR implements the RPC spec for the transaction methods.

The transaction methods include:

Details

The implementation is adjusting the minimum amount of code to maintain both the old RPC, while offering support for the new RPC spec.

  • The spec states that the RPC shall return an error object only if decoding of the transaction fails [here]. However, the RPC will also produce an error if the number of resources hits its limit or the number of active subscriptions exceeds the limit. The dropped event should be used instead, but this requires patching the jsonrpsee first.

  • A validated transaction will enter the tx-pool in the ready or future queues. Therefore, those events are converted to the validated event without modifications.

  • The tx-pool events: Usurped, Dropped, Invalid are converted to the invalid event.

  • The tx-pool FinalityTimeout is converted to the dropped event: "if the block the transaction is included in takes too long to be finalized".

Testing Done

  • Outdated extrinsic
Ok(Object({"error": String("Invalid transaction: Transaction has a bad signature"), "event": String("invalid")}))
  • Invalid extrinsic format
RPC call failed: ErrorObject { code: InvalidParams, message: "0x prefix is missing at line 1 column 3", data: None }
  • Valid extrinsic format with incompatible metadata
Ok(Object({"error": String("Verification error: Runtime error: Execution failed: Runtime panicked: Bad input data provided to validate_transaction: Could not decode `ChargeAssetTxPayment::asset_id`:\n\tunexpected first byte decoding Option\n"), "event": String("invalid")}))
  • Valid extrinsic life cycle
Ok(Object({"event": String("validated")}))
Ok(Object({"block": Object({"hash": String("0x090969a25a0b6d1b094d41364a0e47cbddf5e94b10800077147f05b3c11b138e"), "index": Number(1)}), "event": String("bestChainBlockIncluded")}))
Ok(Object({"block": Object({"hash": String("0x090969a25a0b6d1b094d41364a0e47cbddf5e94b10800077147f05b3c11b138e"), "index": Number(1)}), "event": String("finalized")}))

Next Steps

  • mod tests.rs for transaction API
  • report RPC exceeding resource limits and subscription permits as events
  • optimize sequential events (inBlock 1, inBlock 2, inBock 3) by submitting only the last event (inBlock 3)

Part of https://github.com/paritytech/substrate/issues/12071.

lexnv avatar Sep 22 '22 12:09 lexnv

I'm not familiar with the code, but all your explanations look good to me! :+1: I'm quite happy with it because the transactions pool is far from being simple.

tomaka avatar Sep 22 '22 13:09 tomaka

The spec states that the RPC shall return an error object only if decoding of the transaction fails [here]. However, the RPC will also produce an error if the number of resources hits its limit or the number of active subscriptions exceeds the limit.

Yeah but you could disable this on the server as well if needed. That will be like that until we got backpressure :P

The dropped event should be used instead, but this requires patching the jsonrpsee first.

I don't understand this, it should be sent out as an ordinary notification (which you can do via SubscriptionSink::send) and then just drop the SubscriptionSink? However, I see you use the stream API then it's a bit more awkward to do that but can't you just terminate the stream after that occurs?

niklasad1 avatar Sep 23 '22 08:09 niklasad1

Wrt the jsonrpsee @skunert, @niklasad1: The new RPC spec states we should report limit errors via the subscription's sink as "Dropped" events. Currently, jsonrpsee returns an ObjectError immediately if the server exceeds the resource limits (not enabled for substrate) or reaches the maximum number of subscriptions allowed. This is done here prior to calling the subscription's callback.

We can always come back to this later, patch the RPC server or keep things as they are currently 😄

lexnv avatar Sep 26 '22 11:09 lexnv

bot merge

lexnv avatar Oct 11 '22 08:10 lexnv