RPC queries router #553

tzemanovic · 2022-10-06T11:37:54Z

based on v0.8.1

This PR adds an RPC router macro that is used to replace the manually crafted paths from the previous rpc module. It allows to declare patterns trees with literal string segments, untyped or typed arguments (using FromStr and Display), together with a return type (borsh-encoded in requests) and a handler function. The new RPC definition is here: https://github.com/anoma/namada/blob/tomas/rpc-queries-router/shared/src/ledger/queries/mod.rs#L22-L41. This generates a pub const RPC: Rpc that automatically implements the trait Router. The handler functions are implemented below it and the tests demonstrate RPC's generated API.

Additionally, it allows to declare sub-routers that may be imported from different modules, this is intended to be used to add PoS specific queries (#19), but can be easily re-used for other VPs.

From the same router definition, it can also generate type-safe methods for an async client (added a new feature in shared crate "async-client", disabled by default, enabled in namada_apps build). Some of the client rpc queries have been replaced to use these.

As a note, for this change, I needed to move the protocol module from the apps crate into the shared crate, so that it can be used to implement the dry_run_tx handler. This was a simple move, only required to add rayon dep to the shared crate (this is optional and only enabled when "wasm-runtime" is enabled).

Some implementation details:

There's no runtime router tree unlike in the popular HTTP router crates routefinder, path_tree, route_recognizer and matchit. All definitions are const.
There's no HashMap of parsed str params like in the other router crates. Automatic type-safe handlers routing without any allocations (except for the any typed dynamic params that may allocate when parsed)
The down-side is that it's a declarative macro, I tried to mitigate this with plenty comments in the impl and tests.
Inspired by the very rough crate https://github.com/uniphil/route-rs/, heavily modified to fit our use-case, added the single-level patterns with tree structures, sub-routers support and simpler syntax.
The ugly guts: https://github.com/anoma/namada/blob/tomas/rpc-queries-router/shared/src/ledger/queries/router.rs
An fuller example of the router! syntax in tests: https://github.com/anoma/namada/blob/tomas/rpc-queries-router/shared/src/ledger/queries/router.rs#L755-L782

…raft * namada/tomas/rpc-queries-router: wasm: update checksums changelog: add #553 apps: replace RPC module and its handlers with new queries mod shared: add new queries router macro to replicate handwritten RPC paths add deps for router macro and update `Cargo.lock`s protocol: update imports and add missing rustdoc move ledger's protocol module into shared crate

This adds a new feature "async-client" and "tendermint-rpc" to shared crate that if enabled generate async client code for all router's handler methods, and implements it for tendermint-rpc client, respectively.

* tomas/rpc-queries-router: wasm: update checksums changelog: add #553 apps: replace RPC module and its handlers with new queries mod shared: add new queries router macro to replicate handwritten RPC paths add deps for router macro and update `Cargo.lock`s protocol: update imports and add missing rustdoc move ledger's protocol module into shared crate

brentstone · 2022-10-25T02:21:32Z

shared/src/ledger/queries/mod.rs

+    D: 'static + DB + for<'iter> DBIter<'iter> + Sync,
+    H: 'static + StorageHasher + Sync,
+{
+    require_latest_height(&ctx, request)?;


why require_latest_height here but not in fn storage_value?

it's because storage_prefix doesn't support arbitrary block heights - it relies on DB functionality which only works on the latest committed block

brentstone · 2022-10-25T02:30:31Z

shared/src/ledger/queries/mod.rs

+}
+
+/// For queries that only support latest height, check that the given height is
+/// not different from latest height, otherwise return an error.


Needs a new doc comment, copied from require_latest_height

updated, thanks!

brentstone · 2022-10-25T02:48:11Z

shared/src/ledger/queries/mod.rs

+            height: Option<BlockHeight>,
+            prove: bool,
+        ) -> Result<EncodedResponseQuery, Self::Error> {
+            let data = data.unwrap_or_default();


why not throw errors here?

It's not an error for the data to be None - this is a type conversion from Option<Vec<u8>> to remove the outer layer - unwrap_or_default just turns None into vec![] for easier nicer ergonomics in places where we do use the data

tzemanovic · 2022-10-27T12:47:32Z

There's an issue with the storage value handler as reported by @mateuszjasiuk, thanks! There's extraneous Option<Vec<u8>> re-encoding layer on the value read from storage that causes a breaking change

tzemanovic · 2022-10-28T16:26:27Z

There's an issue with the storage value handler as reported by @mateuszjasiuk, thanks! There's extraneous Option<Vec<u8>> re-encoding layer on the value read from storage that causes a breaking change

I fixed in the follow-up #569 0f43944, because it was too tricky to backport it here as it refactors the shell sub-router. Because of this, this PR shouldn't be used without #569

Namada 0.9.0 * tag 'v0.9.0': Namada 0.9.0 release.sh: list prerequisites in a comment changelog: add #569 queries: fix unused import in wasm build Fixes pgf council vote format Rwnames `type`, adds examples Custom proposal and vote memos in governance spec RPC: fix storage_value to return data as read w/o re-encoding in Option [ci] wasm checksums update router: add `with_options` for handlers that use request/response move the current RPC patterns under "shell" sub-router make fmt Log at INFO by default for namadan Add changelog ledger/queries: comment out `println`s for router path matching dbg ledger/queries: fix require_no_proof doc-string ci: fix workflow name ci: fix mold usage Update README.md some doc comment edits fix: namadan should log at info by default ci: use mold linker changelog: add #632 feat: change native token to nam [ci] wasm checksums update changelog: add #671 add back consensus timeout commit config for abciplus switch to tendermint-rs with consensus timeout wasm: update checksums changelog: add #553 apps: replace RPC module and its handlers with new queries mod shared: add new queries router macro to replicate handwritten RPC paths add deps for router macro and update `Cargo.lock`s protocol: update imports and add missing rustdoc move ledger's protocol module into shared crate ci: invalide cf cache fix broken link split back out the core VPs Modify specs test/e2e/helpers: add a helper to query and parse block height changelog: add #658 client: add a block query to print hash, height and time of a block wasm: update checksums.json [fix]: Fixed the borsh schema for libsecp256k signatures [feat]: Added the recovery id to secp256k signatures [ci] wasm checksums update feat: remove nft

- changed to type of the proof in query Response for more convenient type Namada 0.9.0 * tag 'v0.9.0': Namada 0.9.0 release.sh: list prerequisites in a comment changelog: add #569 queries: fix unused import in wasm build Fixes pgf council vote format Rwnames `type`, adds examples Custom proposal and vote memos in governance spec RPC: fix storage_value to return data as read w/o re-encoding in Option [ci] wasm checksums update router: add `with_options` for handlers that use request/response move the current RPC patterns under "shell" sub-router make fmt Log at INFO by default for namadan Add changelog ledger/queries: comment out `println`s for router path matching dbg ledger/queries: fix require_no_proof doc-string ci: fix workflow name ci: fix mold usage Update README.md some doc comment edits fix: namadan should log at info by default ci: use mold linker changelog: add #632 feat: change native token to nam [ci] wasm checksums update changelog: add #671 add back consensus timeout commit config for abciplus switch to tendermint-rs with consensus timeout wasm: update checksums changelog: add #553 apps: replace RPC module and its handlers with new queries mod shared: add new queries router macro to replicate handwritten RPC paths add deps for router macro and update `Cargo.lock`s protocol: update imports and add missing rustdoc move ledger's protocol module into shared crate ci: invalide cf cache fix broken link split back out the core VPs Modify specs test/e2e/helpers: add a helper to query and parse block height changelog: add #658 client: add a block query to print hash, height and time of a block wasm: update checksums.json [fix]: Fixed the borsh schema for libsecp256k signatures [feat]: Added the recovery id to secp256k signatures [ci] wasm checksums update feat: remove nft

…wards * origin/main: (632 commits) ci: use commit hash instead of pr number ci: fix binaries sha changelog: add #720 fix: rename correctly in debug wasm build Namada 0.9.0 release.sh: list prerequisites in a comment changelog: add #569 queries: fix unused import in wasm build Fixes pgf council vote format Rwnames `type`, adds examples Custom proposal and vote memos in governance spec RPC: fix storage_value to return data as read w/o re-encoding in Option [ci] wasm checksums update router: add `with_options` for handlers that use request/response move the current RPC patterns under "shell" sub-router make fmt Log at INFO by default for namadan Add changelog ledger/queries: comment out `println`s for router path matching dbg ledger/queries: fix require_no_proof doc-string ci: fix workflow name ci: fix mold usage Update README.md some doc comment edits fix: namadan should log at info by default ci: use mold linker changelog: add #632 feat: change native token to nam [ci] wasm checksums update changelog: add #671 add back consensus timeout commit config for abciplus switch to tendermint-rs with consensus timeout wasm: update checksums changelog: add #553 apps: replace RPC module and its handlers with new queries mod shared: add new queries router macro to replicate handwritten RPC paths add deps for router macro and update `Cargo.lock`s protocol: update imports and add missing rustdoc move ledger's protocol module into shared crate ci: invalide cf cache fix broken link split back out the core VPs Modify specs test/e2e/helpers: add a helper to query and parse block height changelog: add #658 client: add a block query to print hash, height and time of a block Namada 0.8.1 changelog: add #510 [feat]: Make process proposal stateless test/e2e: fix node_connectivity test test/e2e: update expected string for non-validator node [fix]: Removed unnecessary patches in wasm [feat]: Patched tendermint-rs and ibc-rs to compatible versions app: set "namada" set default bin for `cargo run` wasm: update checksums.json [fix]: Fixed the borsh schema for libsecp256k signatures [feat]: Added the recovery id to secp256k signatures [ci] wasm checksums update feat: remove nft fix: rename anoma to namada, remove author print CI: add workaround for release build missing git tag Namada 0.8.0 changelog: add #547 wasm: update checksums [chore]: Lints and formatting [fix]: Fixed proof specs for non-existence proofs [fix]: Update the main tree when deleting a key [fix]: Fixed last ibc unit test [fix]: Fixed some tests. [feat]: Refactored the merkle tree into a more maintainable multi-store build: pin abciplus library revisions wasm: update checksums.json ci: standardize on one tendermint hash formatting changes change `L_{NAM}` -> `L_{PoS}` for clarity and consistency change inflation `I` to have units of tokens per epoch fix term ordering in error calculation Remove other `:=` in favor of `=` distinguish new and old inflation rates with a prime fix md rendering correct the shielded pool error calculation change storing of error to storing of token ratios (same procedure) fix: `I` is percent per annum distinguish between `K` in storage and `K` as intermediate value in inflation calculation Use `I` for inflation rates, as `R` reserved for locked ratios specify some default values, other small edits release: add scripts/release.sh release: add release.toml to wasm workspace and wasm_for_tests release: don't tag from cargo release release: update release.toml for cargo-release 0.21.4 wasm: remove tx_from_intent build wasm: repair wasm checksums Add changelog Add changelog Fix TransactionGasExceededed typo changelog: add #594 shared/storage: fix the height recorded for a new epoch Apply suggestions from code review changelog: add #590 wasm/tx_bond: fix delegation detection to compare source and validator ...

tzemanovic force-pushed the tomas/rpc-queries-router branch 6 times, most recently from 4f6cb64 to b4ab042 Compare October 7, 2022 12:32

tzemanovic added a commit that referenced this pull request Oct 7, 2022

changelog: add #553

4a169ea

tzemanovic mentioned this pull request Oct 7, 2022

RPC shell sub-router #569

Merged

tzemanovic changed the title ~~rpc queries router~~ RPC queries router Oct 7, 2022

tzemanovic marked this pull request as ready for review October 7, 2022 16:14

juped added this to the v0.9.0 milestone Oct 17, 2022

tzemanovic added a commit that referenced this pull request Oct 17, 2022

changelog: add #553

c7aa74c

tzemanovic force-pushed the tomas/rpc-queries-router branch 2 times, most recently from de260f9 to 08c7675 Compare October 18, 2022 09:22

juped added the merged to draft label Oct 18, 2022

tzemanovic added 7 commits October 24, 2022 11:58

move ledger's protocol module into shared crate

9b5a8d9

protocol: update imports and add missing rustdoc

84777a2

add deps for router macro and update Cargo.locks

2b36531

This adds a new feature "async-client" and "tendermint-rpc" to shared crate that if enabled generate async client code for all router's handler methods, and implements it for tendermint-rpc client, respectively.

shared: add new queries router macro to replicate handwritten RPC paths

0c9b0bc

apps: replace RPC module and its handlers with new queries mod

c593917

changelog: add #553

66c632c

wasm: update checksums

6813526

tzemanovic force-pushed the tomas/rpc-queries-router branch from 08c7675 to 6813526 Compare October 24, 2022 10:40

tzemanovic requested a review from brentstone October 24, 2022 14:57

tzemanovic mentioned this pull request Oct 24, 2022

PoS integration #677

Closed

9 tasks

brentstone reviewed Oct 25, 2022

View reviewed changes

brentstone and others added 2 commits October 25, 2022 14:08

some doc comment edits

3b0117e

ledger/queries: fix require_no_proof doc-string

c1d8f04

ledger/queries: comment out printlns for router path matching dbg

ddc62be

make fmt

a16180f

juped merged commit 3a8700b into main Oct 31, 2022

juped deleted the tomas/rpc-queries-router branch October 31, 2022 18:04

tzemanovic mentioned this pull request Jan 13, 2023

RPC queries - refactor library code #66

Closed

phy-chain pushed a commit to phy-chain/namada that referenced this pull request Mar 1, 2024

fix: fix typos (anoma#553)

6a78023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RPC queries router #553

RPC queries router #553

tzemanovic commented Oct 6, 2022 •

edited

Loading

brentstone Oct 25, 2022

tzemanovic Oct 26, 2022

brentstone Oct 25, 2022

tzemanovic Oct 27, 2022

brentstone Oct 25, 2022

tzemanovic Oct 26, 2022

tzemanovic commented Oct 27, 2022

tzemanovic commented Oct 28, 2022

RPC queries router #553

RPC queries router #553

Conversation

tzemanovic commented Oct 6, 2022 • edited Loading

brentstone Oct 25, 2022

Choose a reason for hiding this comment

tzemanovic Oct 26, 2022

Choose a reason for hiding this comment

brentstone Oct 25, 2022

Choose a reason for hiding this comment

tzemanovic Oct 27, 2022

Choose a reason for hiding this comment

brentstone Oct 25, 2022

Choose a reason for hiding this comment

tzemanovic Oct 26, 2022

Choose a reason for hiding this comment

tzemanovic commented Oct 27, 2022

tzemanovic commented Oct 28, 2022

tzemanovic commented Oct 6, 2022 •

edited

Loading