#256 JSON-RPC Package (& Commons) #261
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sekimondre
commented
Jun 6, 2022
Sources/JSONRPC/RPCRequest.swift
Outdated
| throw DecodingError.dataCorruptedError( | ||
| forKey: .params, | ||
| in: container, | ||
| debugDescription: "") |
|
Looks, very clean and thoroughly tested, I just wonder if it is not too complex fro our needs but looking forward how it will work consumed by SDKs. |
llbartekll
reviewed
Jun 6, 2022
Comment on lines
37
to
+39
| .target( | ||
| name: "WalletConnectUtils", | ||
| dependencies: ["Commons"]), |
There was a problem hiding this comment.
Can we define a difference between WalletConnectUtils and a Commons ?
There was a problem hiding this comment.
WalletConnectUtils should have types related to WalletConnect domain rules, Commons should be always agnostic of any WalletConnect implementation. I think we could even rename WalletConnectUtils to WalletConnectCore and, over time, extract the components (storage, logging) to their own packages.
llbartekll
approved these changes
Jun 6, 2022
flypaper0
approved these changes
Jun 7, 2022
|
We could close #155 after that changes? |
Absolutely |
Sure, I'll try doing something on a next PR |
Merged
flypaper0
added a commit
that referenced
this pull request
Jun 9, 2022
* Cleanup Service + SequenceStore refactor (#241) * Cleanup service * Clean sessionToPairingTopic * SequenceStore refactor * Rename KeyValueStore -> CodableStore * Deliver an invite (#254) * Add Chat target, split packages * savepoint * Update networking interactor to decode unencrypted messages * pass on invite test * restructure chat's project folder * Add engine storages * extract storage domain identifiers * update logging * fix schemas * Update style * Add publishers to Chat * rename kv store to codable strore * UI tests (#242) * Pairing testcase * Ping testcase * ApproveSessionExistingPairing test case * Unused extensions deleted * Renamed to RegressionTests * UITests step on CI * CleanLaunch instead of deleting app * Fix test on Real device * Launch App fix * Approve engine refactor (#260) * Approve method moved ApproveEngine * Reject and wcSessionSubscriptions for ApproveEngine * Private methods moved in extension * ApproveEngine errors handlers * Try on reject * ApproveEngine moved to callbacks * Session Settle moved to approve Engine * onProposeResponse subscription removed * Reject by proposalId * Settle moved to approve * ApproveEngine moved to Controller folder * typealias removed * TODO for SettleEngine * #256 JSON-RPC Package (& Commons) (#261) * Add Chat target, split packages * savepoint * restructure chat's project folder * fix schemas * Add JSONRPC and Commons packages * Moved AnyCodable to Commons * Fixed test import * Reintroduces either type * Add request and response types * Add simple response decode tests * Add response ID parsing tests * Fixed tests typo * Improved response round trip coding test * Error response decoding tests * Invalid response decode tests * Enabled code coverage for library * Response decoding tests for structured result values * Add flexible initializers with tests * Add descriptions to errors thrown in response decoding * Renamed response internalResult to outcome * Basic RPC request decoding tests * Tests for request empty cases and corner cases * Add flexible inits for requests * Add identifier generation inits * Joined request notification extensions * Renamed files * Implemented default JSONRPC error cases * Declared RPCRequestConvertible as public * Remove rebase artifacts * Added debug description to request param primitives error Co-authored-by: Bartosz Rozwarski <bartus000@gmail.com> Co-authored-by: André Vants <MisterVants@users.noreply.github.com>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This PR introduces a full compliant JSON-RPC 2.0 interface for request and responses. Specs can be found at jsonrpc.org
What changed
CommonsandJSONRPCpackages.RPCRequestandRPCResponsecompliant with JSON-RPC 2.0 specifications.RPCRequestConvertibleto facilitate decouple of protocol methods from RPC requests.AnyCodabletype moved to Commons package.Eithertype.Notes
Types were introduced in a way to interfere as little as possible with current SDK code. Current request objects can be migrated over time, and ID generation logic can be wrapped over the RPC models.
The JSONRPC types can be considered foundational code and are build with the intention to be flexible, solid, and allow use in other types of application. Feedback over the API and its usage is appreciated.