Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: filtered queries in contracts #4544

Merged
merged 2 commits into from May 16, 2024

Conversation

DCNick3
Copy link
Contributor

@DCNick3 DCNick3 commented May 2, 2024

Description

  • remove QueryWithParameters
  • move all parameters to ClientQueryPayload (was QueryPayload before, stored inside SignedQuery); SignedQuery now equivalent to QueryWithParameters<SignedQuery>. All the parameters are now scale-encoded and versioned instead of being passed as query parameters
  • make SmartContractQuery, which is now equivalent to QueryWithParameters<QueryBox> that was used before, but has filtering support
  • move all the post-processing (filterting, sorting, pagination, batching) to a single function (LazyQueryOutput::apply_postprocessing), introduce a new type for a post-processed query output
  • add an integration test using the filtering (ProcessedQueryOutput)

Linked issue

Closes #4423

Checklist

  • make CI pass

@github-actions github-actions bot added the api-changes Changes in the API for client libraries label May 2, 2024
@mversic mversic changed the title Filtered queries in contracts feat: filtered queries in contracts May 2, 2024
@DCNick3 DCNick3 force-pushed the filtered-queries-in-contracts branch from e900abc to 469fdd3 Compare May 2, 2024 13:15
@DCNick3 DCNick3 self-assigned this May 2, 2024
@DCNick3 DCNick3 force-pushed the filtered-queries-in-contracts branch from 469fdd3 to 6b8abfc Compare May 2, 2024 13:58
@Erigara Erigara self-assigned this May 2, 2024
Erigara
Erigara previously approved these changes May 3, 2024
Copy link
Contributor

@Erigara Erigara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

0x009922
0x009922 previously approved these changes May 10, 2024
torii/src/routing.rs Outdated Show resolved Hide resolved
smart_contract/src/lib.rs Show resolved Hide resolved
core/src/smartcontracts/isi/query.rs Outdated Show resolved Hide resolved
@mversic mversic requested a review from Erigara May 13, 2024 05:52
data_model/src/query/mod.rs Show resolved Hide resolved
smart_contract/src/lib.rs Show resolved Hide resolved
@DCNick3 DCNick3 force-pushed the filtered-queries-in-contracts branch 3 times, most recently from fccbebf to f776112 Compare May 14, 2024 12:47
DCNick3 added a commit to DCNick3/iroha-2-docs that referenced this pull request May 14, 2024
Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>
DCNick3 added a commit to hyperledger/iroha-2-docs that referenced this pull request May 15, 2024
Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>
0x009922
0x009922 previously approved these changes May 15, 2024
Erigara
Erigara previously approved these changes May 15, 2024
@DCNick3 DCNick3 dismissed stale reviews from Erigara and 0x009922 via cf141b3 May 15, 2024 11:55
@DCNick3 DCNick3 force-pushed the filtered-queries-in-contracts branch from f776112 to cf141b3 Compare May 15, 2024 11:55
mversic
mversic previously approved these changes May 15, 2024
Erigara
Erigara previously approved these changes May 15, 2024
@DCNick3 DCNick3 dismissed stale reviews from Erigara and mversic via 142bb7e May 15, 2024 12:47
@DCNick3 DCNick3 force-pushed the filtered-queries-in-contracts branch 2 times, most recently from 142bb7e to a99c2c2 Compare May 15, 2024 13:28
@DCNick3
Copy link
Contributor Author

DCNick3 commented May 15, 2024

I changed the integration test to query for asset definitions instead of accounts, as copying pubkey ids is probably a bad idea and test_samples is not very easy to make work in no_std due to once_cell requiring some form of synchronization.

@mversic
Copy link
Contributor

mversic commented May 16, 2024

and test_samples is not very easy to make work in no_std

I'm not a big fan of having test_samples shared across crates

@mversic mversic requested review from Erigara and 0x009922 May 16, 2024 05:51
Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>
Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>
@DCNick3 DCNick3 force-pushed the filtered-queries-in-contracts branch from a99c2c2 to a7f498e Compare May 16, 2024 07:15
@DCNick3
Copy link
Contributor Author

DCNick3 commented May 16, 2024

with_coverage failure is... weird

Hope it's a one off
image

@DCNick3 DCNick3 merged commit 663803c into hyperledger:main May 16, 2024
13 of 15 checks passed
@nxsaken nxsaken changed the title feat: filtered queries in contracts feat!: filtered queries in contracts May 17, 2024
@s8sato
Copy link
Contributor

s8sato commented May 17, 2024

@DCNick3 @mversic You can refactor what test_samples provides if you like. It only needs to be deterministic to agree on keys e.g. KeyPair::from_seed could be utilized. I didn't think of it at the time of #4411

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes Changes in the API for client libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]: Query filtering API isn't available in smart-contracts
5 participants