Support batching multiple RPC requests #455
Conversation
… into feat/443-batch-multiple-rpc-requests
….com/software-mansion/starknet-jvm into feat/443-batch-multiple-rpc-requests
lib/src/main/kotlin/com/swmansion/starknet/provider/rpc/BuildJsonHttpDeserializer.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/com/swmansion/starknet/provider/rpc/BuildJsonHttpDeserializer.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/com/swmansion/starknet/provider/rpc/JsonRpcProvider.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/com/swmansion/starknet/service/http/HttpRequest.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/com/swmansion/starknet/service/http/HttpRequest.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/com/swmansion/starknet/provider/rpc/JsonRpcProvider.kt
Outdated
Show resolved
Hide resolved
lib/src/test/kotlin/starknet/provider/response/JsonRpcResponseTest.kt
Outdated
Show resolved
Hide resolved
lib/starknet-jvm.md
Outdated
| public class Main { | ||
| public static void main(String[] args) { | ||
| // Create a provider for interacting with Starknet | ||
| Provider provider = new JsonRpcProvider("https://example-node-url.com/rpc"); |
There was a problem hiding this comment.
Just making sure, have you tried running this example code?
It seems to me that this might not work, as provider is of Provider interface, and Provider interface (I assume intentionally) does not support batchRequests.
There was a problem hiding this comment.
Correct, batchRequests is not supported by Provider. It is a custom method and not an equivalent of specified Starknet endpoint. That's why I decided not to include it in Provider.
However, it may be potentially confusing for users that they cannot use batchRequests on an instance of Provider.
Wdyt?
Regarding the test - I've updated sample code and replaced Provider with JsonRpcProvider 👍
There was a problem hiding this comment.
It is a custom method and not an equivalent of specified Starknet endpoint. That's why I decided not to include it in Provider.
I would say it's JSON-RPC specific method. Previously, we also had GatewayProvider which likely would not support batching. However, currently afaik there are no ways to communicate with Starknet other than JSON-RPC, so theoretically all providers are "JSON-RPC" providers, and the original naming was retained mainly for backward compatibility.
The way I see it, we either:
- Leave batching only in
JsonRpcProvider, because it's JSON-RPC specific. - Push batching up to
Providerinterface to allow users to keep using theProvideras an interface.
It seems to me 2. offers broader utility, however, it slightly breaks abstraction by introducing some underlying request and response mechanics to the provider interface.
WDYT?
There was a problem hiding this comment.
While I generally uphold the principle of not violating the level of abstraction, the deviation in this case is rather minimal, making the 2. option possibly acceptable 👍 . Consequently, implementing Provider interface in JsonRpcpProvider currently seems unnecessary and doesn't add significant value. Do we want to keep it, if all providers are "JSON-RPC" providers?
WDYT?
There was a problem hiding this comment.
Do we want to keep it, if all providers are "JSON-RPC" providers?
Discussed offline.
the deviation in this case is rather minimal, making the 2. option possibly acceptable
Another thing that concerns me is that we'd introduce HttpRequest<T> to the method signature of an interface, which is not abstract unlike `Request and is JSON-RPC specific.
I don't have a strong say on what's the best approach here, @THenry14 wdyt?
There was a problem hiding this comment.
No strong opinion either, but I guess it makes a bit more sense to me to keep it in JsonRpcProvider
…cProvider.kt Co-authored-by: Maksim Zdobnikau <43750648+DelevoXDG@users.noreply.github.com>
…quest.kt Co-authored-by: Maksim Zdobnikau <43750648+DelevoXDG@users.noreply.github.com>
…cProvider.kt Co-authored-by: Maksim Zdobnikau <43750648+DelevoXDG@users.noreply.github.com>
…Test.kt Co-authored-by: Maksim Zdobnikau <43750648+DelevoXDG@users.noreply.github.com>
… block hash and block tag` test
….com/software-mansion/starknet-jvm into feat/443-batch-multiple-rpc-requests
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #455 +/- ##
==========================================
- Coverage 71.12% 69.62% -1.50%
==========================================
Files 75 75
Lines 3338 3315 -23
Branches 367 365 -2
==========================================
- Hits 2374 2308 -66
- Misses 798 850 +52
+ Partials 166 157 -9 ☔ View full report in Codecov by Sentry. |
lib/src/main/kotlin/com/swmansion/starknet/provider/rpc/BuildJsonHttpDeserializer.kt
Outdated
Show resolved
Hide resolved
… into feat/443-batch-multiple-rpc-requests
… into feat/443-batch-multiple-rpc-requests
Describe your changes
batchRequestsmethod (vararg and list versions)JsonRpcRequestdata classBatchHttpRequestclassProviderTest:batch call contract with block hash and block tagbatch get transactionsJsonRpcResponseTest:rpc provider parses batch getTransactionStatusrpc provider parses batch callContractrpc provider parses batch response with incorrect orderJsonRpcRequestdata class representing single RPC requeststarknet-jvm.mdreadme with proper examples ofbatchRequestsusageLinked issues
Closes #443
Breaking changes