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

Support Iroha 2.0.0-rc.1 #44

Draft
wants to merge 43 commits into
base: master
Choose a base branch
from
Draft

Support Iroha 2.0.0-rc.1 #44

wants to merge 43 commits into from

Conversation

0x009922
Copy link
Contributor

@0x009922 0x009922 commented Aug 8, 2024

This is a from-scracth re-implementation of the backend for Iroha v2.0.0-rc.1.

Features:

  • SQL-driven
  • OpenAPI auto-generated schema, available at /scalar

Close #43

SQLite-based solution

The Explorer keeps an up-to-date in-memory SQLite database representing current state of Iroha (blocks/txs history + current domains/accounts/assets).

This solution isn't scalable, as it re-creates an entire database on each small change. There are a few reasons for it, however:

  1. Iroha Query API isn't sufficient to serve Explorer needs. Upstream changes are slow and it isn't clear how they should look like.
  2. By using SQL now, backend is able to provide necessary features to build a complete frontend.
  3. By using SQL now, it became a bit clearer what changes we want from Iroha

Related upstream issues/PRs

Checklist

  • 2.0.0-rc.1 is released on crates.io (so that we can use it in Cargo.toml directly)
  • Update README
  • Update CI

@0x009922 0x009922 self-assigned this Aug 8, 2024
@0x009922
Copy link
Contributor Author

0x009922 commented Aug 9, 2024

Paper trail: occurring challenges

Total count

I can't see anymore a way to get a total count of items when fetching lists. In Explorer we need to let users know how many pages of data there is (domains, accounts, etc).

Blocks predicates

Blocks predicates are empty for now. In Explorer we need to fetch a certain block by its height or hash. While fetching by height could be done via tricks with pagination, finding by hash is not possible in an efficient way.

upd: It is the reason why FindBlockHeaderByHash and FindTransationByHash singular queries still exist.

Transactions order

While FindBlocks returns blocks in a reverse order (height desc) and it makes sense, FindTransactions does not (haven't checked yet, only analysed code). I think transactions should also be sorted creation_time desc by default.

Inconsistent Pagination struct

  • Is not constructable without the transparent_api feature. Ugly workaround via serde.
  • start is Option<NonZero<u64>>, where None effectively means zero. Just make it u64 with 0 as default.
  • start is u64, while limit is u32. The upper limit of these values is the same - maximum total amount of items returned. So they should be both either u64 or u32

upd: start is now offset.

hyperledger-iroha/iroha#4978

Data model immutability

Given the immutability of the structures, it is impossible to decompose them in most cases. The only way to access them is to get immutable references. Explorer re-wraps everything in its own structures. It could be done either by lots of clones, or by lots of lifetimes. It seems a bit insane given that in the context of Explorer there is not point of holding original data model structures and making its reflections for api responses. It would be convenient to own and move internal data directly.

It could be workarounded with the transparent_api feature, but it is supposed to be for internal use only.

upd: BlockSignature and TransactionSignature - not possible to access internals.

@0x009922
Copy link
Contributor Author

More issues:

  • Since FindBlockHeaders returns just BlockHeaders, which doesn't contain block's hash, it is useless for Explorer now. Currently I use FindBlocks, but it also retrieves all block transactions, which amount might be huge and it is not always needed eagerly. Solution might be supporting something similar to the output of FindTransactions, which doesn't give just a transaction, but an additional field with a block hash.
  • Thus, there is also a missing API - filtering transactions by block hash.

WIP:

- assets/assets definitions queries/endpoints
- instructions endpoints
Assets and Instructions endpoints are not
yet implemented
- add `TransactionStatus` enum
- remove `TransactionInList`, use `TransactionBase` & `TransactionDetailed`
- add `status` to `TransactionBase`, remove `error`
- rename `error` to `rejection_reason`
- add filter by `status` to `GET /transactions`
- for `GET /instructions`, only return those from committed transactions, without an option to configure it
@0x009922
Copy link
Contributor Author

0x009922 commented Sep 3, 2024

Recent status of this PR.

SQL-based backend

Decided to make a temporary SQL solution instead of relying on Iroha Query API. This both helps to implement everything we want in the explorer, and also highlights what changes are desirable from Iroha.

Currently backend uses a file-based pre-created static SQLite database. My plan is to incorporate a functionality into the program that will "scrap" Iroha on updates and re-create a SQLite database in-memory. Not a scalable solution, but should work fine for not-very-large Iroha.

@0x009922
Copy link
Contributor Author

0x009922 commented Sep 3, 2024

@VladislavPopovSR please note recent changes:

  • Fixed filtering txs by block hash
  • Added support to filter instructions by their kind and the authority. This allows to have a table of instructions on the account details page.
  • When getting instructions, only committed ones are returned. It is not configurable. I thought it doesn't make sense for now to see rejected instructions - users can see rejected transactions already.
  • Added status field to transactions, added filter by status, removed error bool flag, added rejection_reason nullable field for detailed transaction object.

- use block `height` as a primary key
- replace transaction `block_hash` with `block`
- update `GET /transactions` - replace query param `block_hash` with `block`
- return _all_ instructions again, not only within successful txs
  - add instruction `transaction_status` field
  - TODO: filter by `transaction_status`
@0x009922
Copy link
Contributor Author

Recent updates related to frontend:

  • Transaction:
    • rename instructions to executable
    • remove block_hash, add block (height)
  • Instruction:
    • add transaction_status
  • GET /transactions: remove block_hash query param, add block
  • GET /instructions: don't filter only committed ones, return all. TODO: add transaction_status query param filter

cc: @VladislavPopovSR

@0x009922
Copy link
Contributor Author

This work is sort of finished for now. I've updated PR's description.

I think we should undraft this PR once Iroha "MVP" (release candidate 1) is released on crates.io.

@0x009922
Copy link
Contributor Author

0x009922 commented Sep 24, 2024

GET /instructions now supports two new filters: by block and by transaction_status.

cc: @VladislavPopovSR

@0x009922
Copy link
Contributor Author

Removed consensus_estimation field from Block schema.

cc: @VladislavPopovSR

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to Iroha 2.0.0-rc.1
2 participants