Refactoring of the `graphql-api` service to use ports for `Database` #858

ControlCplusControlV · 2022-12-20T20:05:20Z

Refs #809
Closes: #907

It is the refactoring for the graphql-api service to use ports instead of direct work with Database. The service defines minimal database functionality and builds its functionality on top of it to process GrapQL requests.

Right now, this functionality is represented by {}QueryContext structures with the public methods. It is done to make the migration easier.
But later, we will need to move this functionality into corresponding traits and implement it directly for the DatabaseTemp type. Like:

impl {}QueryContext for DatabaseTemp {...}

or

impl<D: DatabasePort> {}QueryContext for D {...}

With traits, we can mock them and improve our tests.
The PR contains the following changes:

Renamed Transactional into Transaction.
Added Transactional trait with transaction method.
Used BlockId instead of Bytes32 in Database.
Created ports for block.rs, tx.rs, message.rs, balance.rs, coin.rs and updated corresponding schema.
Added and used in ports BoxedIter instead of associated types(In most cases they were Box<dyn> too).
Almost removed usage of fuel_core::database from schema. Only dap.rs uses it right now for VMDatabase.
Introduced CompressedCoin and Coin(contains UtxoId). CompressedCoin is used during work with the database, Coin in external API.
Moved resources into fuel-core-type and simplified it by removing generics.
Tried to re-work dap.rs but the change is big and also related to fuel-core-executor, so will do it in a separate PR.
Updated Transactional to support Transactional<dyn Database>

Before we can extract grapgql-api into its crate, we need:

Add ports for other services.
Add iteration abstraction into fuel-core-storage.
Solve the problem with DatabaseVM(The solution should also fit for fuel-core-executor).

crates/fuel-core/src/query/block.rs

crates/fuel-core/src/query/chain.rs

crates/fuel-core/src/query/coin.rs

crates/fuel-core/src/schema/block.rs

crates/fuel-core/src/schema/chain.rs

crates/fuel-core/src/schema/coin.rs

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>

.github/workflows/ci.yml

- Renamed `Transactional` into `Transaction`. - Added `Transactional` trait with `transaction` method. - Used `BlockId` instead of `Bytes32` in `Database`. - Created ports for `block.rs`, `tx.rs`, `message.rs`, `balance.rs`, `coin.rs` and updated corresponding schema. - Added and used in ports `BoxedIter` instead of associated types(In most cases they were `Box<dyn>` too). - Almost removed usage of fuel_core::database from schema. Only `dap.rs` uses it right now for `VMDatabase`. - Introduced `CompressedCoin` and `Coin`(contains `UtxoId`). `CompressedCoin` is used during work with the database, `Coin` in external API. - Moved resources into `fuel-core-type` and simplified it by removing generics. - Tried to re-work `dap.rs` but the change is big and also related to `fuel-core-executor`, so will do it in a separate PR. - Updated `Transactional` to support `Transactional<dyn Database>` Co-authored-by: ControlCplusControlV <44706811+ControlCplusControlV@users.noreply.github.com>

Voxelot · 2023-01-11T08:57:37Z

This is getting to be a very large PR, what's left todo on it? It may be good to work on getting this merged before more refactoring.

xgreenx · 2023-01-11T16:39:41Z

It is in the stage when almost all work with Database is replaced with ports(except VMDatabase). I also vote to merge it=)

xgreenx · 2023-01-11T17:01:01Z

crates/fuel-core/src/query/message_proof.rs

+        .transaction_status(&transaction_id)
+        .into_api_result::<TransactionStatus, StorageError>()?
+    {
+        Some(TransactionStatus::Success { block_id, .. }) => block_id,


I want to highlight that here I changed the business logic, now we don't allow verification for failed transactions.

This is probably ok since messages are only set by the VM currently. However there's nothing specifically preventing failed scripts from having a valid message output in the future.

ControlCplusControlV · 2023-01-12T00:44:13Z

I have changes fixing the TempDatabase vs Database issue, so I would prefer to wait until that is in before merging. Awaiting some work around the debugger before committing (since its a shared PR), but discussing with @Voxelot soon

Removed unused `OwnedTransactionIndexCursor` from the schema. Extracted `dap.rs` into the `fuel-core` level. Renamed `TempDatabase` into `Database`.

ControlCplusControlV · 2023-01-12T01:59:08Z

PR should be good for review now that dap is separated out and instances of TempDatabase have been renamed

Voxelot · 2023-01-13T20:47:02Z

crates/fuel-core/src/graphql_api/ports.rs

+
+/// The database port expected by GraphQL API service.
+#[cfg(test)]
+pub trait DatabasePort:


[nit] why not just define a trait extension in tests?

Voxelot · 2023-01-13T20:52:04Z

crates/fuel-core/src/schema/chain.rs

+        let query = BlockQueryContext(ctx.data_unchecked());
+
+        let latest_block = query.latest_block()?.into();
+        Ok(latest_block)
    }

    async fn base_chain_height(&self) -> U64 {


just realized we forgot to update this api after finishing the relayer

Implemented base chain height. Removed duplication of `DatabasePort` for tests.

Voxelot · 2023-01-13T22:44:13Z

crates/fuel-core/src/database/relayer.rs

@@ -21,7 +21,7 @@ use fuel_core_types::{

 #[async_trait::async_trait]
 impl RelayerDb for Database {
-    async fn insert_message(


ControlCplusControlV requested a review from a team December 20, 2022 20:05

ControlCplusControlV added the graphql-api Affects API of the GraphQL label Dec 20, 2022

ControlCplusControlV self-assigned this Dec 20, 2022

xgreenx reviewed Dec 20, 2022

View reviewed changes

Base automatically changed from teamwork/removing_channels to master December 21, 2022 03:25

ControlCplusControlV and others added 8 commits December 21, 2022 13:36

refactor coin query

0f4a1c1

migrated chain singular queries

135f2b6

moved over block query

353c12e

Refactored all single queries

deb15d3

moved traits out of schema

ce662f4

moved traits out of schema

b55d94e

Update crates/fuel-core/src/query/block.rs

30756fb

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>

fmt:

1b0b321

Voxelot force-pushed the controlc/gql_refactor branch from 51311f3 to 1b0b321 Compare December 21, 2022 21:37

addressed pr comments

34a256f

ControlCplusControlV requested review from xgreenx and a team December 21, 2022 22:41

xgreenx and others added 4 commits December 23, 2022 01:48

Merge branch 'master' into controlc/gql_refactor

6ad858a

fixed issue

a9a5a93

Merge branch 'master' into controlc/gql_refactor

7db5330

fixed updates

adf222d

Voxelot reviewed Dec 23, 2022

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

ControlCplusControlV added 2 commits December 23, 2022 12:03

remove ci change

b93e77a

remove ci change

8390684

ControlCplusControlV requested review from a team and Voxelot December 23, 2022 19:05

ControlCplusControlV and others added 4 commits December 23, 2022 15:53

fixed contract query

276dd49

Merge branch 'master' into controlc/gql_refactor

5c6cc94

core

27c4e68

xgreenx reviewed Jan 11, 2023

View reviewed changes

xgreenx previously approved these changes Jan 11, 2023

View reviewed changes

xgreenx changed the title ~~Controlc/gql refactor~~ Refactoring of the graphql-api service to use ports for Database Jan 11, 2023

xgreenx self-assigned this Jan 11, 2023

xgreenx requested a review from a team January 11, 2023 17:12

Merge branch 'master' into controlc/gql_refactor

0d97e98

Leave dap.rs in the fuel-core level for now (#899)

ade0a04

Removed unused `OwnedTransactionIndexCursor` from the schema. Extracted `dap.rs` into the `fuel-core` level. Renamed `TempDatabase` into `Database`.

xgreenx dismissed their stale review via ade0a04 January 12, 2023 01:58

xgreenx previously approved these changes Jan 12, 2023

View reviewed changes

xgreenx requested a review from a team January 12, 2023 01:58

Merge branch 'master' into controlc/gql_refactor

b506822

Voxelot reviewed Jan 13, 2023

View reviewed changes

Merge branch 'master' into controlc/gql_refactor

e36125b

Voxelot mentioned this pull request Jan 13, 2023

Make graphql api for base chain height use da_height from relayer #907

Closed

Address comments in teh PR:

af0aebd

Implemented base chain height. Removed duplication of `DatabasePort` for tests.

xgreenx dismissed their stale review via af0aebd January 13, 2023 22:40

xgreenx force-pushed the controlc/gql_refactor branch from ca3ee34 to af0aebd Compare January 13, 2023 22:40

Voxelot reviewed Jan 13, 2023

View reviewed changes

xgreenx added 2 commits January 13, 2023 22:45

Forgot to remove async_trait

68c8424

Merge branch 'master' into controlc/gql_refactor

6a28999

Voxelot approved these changes Jan 13, 2023

View reviewed changes

Voxelot merged commit 8575ee0 into master Jan 14, 2023

Voxelot deleted the controlc/gql_refactor branch January 14, 2023 00:18

xgreenx mentioned this pull request Jan 31, 2023

Proposal to move out GraphQL API from fuel-core to fuel-indexer #588

Open

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring of the `graphql-api` service to use ports for `Database` #858

Refactoring of the `graphql-api` service to use ports for `Database` #858

ControlCplusControlV commented Dec 20, 2022 •

edited by xgreenx

Loading

Voxelot commented Jan 11, 2023

xgreenx commented Jan 11, 2023

xgreenx Jan 11, 2023

Voxelot Jan 13, 2023

ControlCplusControlV commented Jan 12, 2023

ControlCplusControlV commented Jan 12, 2023

Voxelot Jan 13, 2023 •

edited

Loading

Voxelot Jan 13, 2023 •

edited

Loading

Voxelot Jan 13, 2023

Refactoring of the graphql-api service to use ports for Database #858

Refactoring of the graphql-api service to use ports for Database #858

Conversation

ControlCplusControlV commented Dec 20, 2022 • edited by xgreenx Loading

Voxelot commented Jan 11, 2023

xgreenx commented Jan 11, 2023

xgreenx Jan 11, 2023

Choose a reason for hiding this comment

Voxelot Jan 13, 2023

Choose a reason for hiding this comment

ControlCplusControlV commented Jan 12, 2023

ControlCplusControlV commented Jan 12, 2023

Voxelot Jan 13, 2023 • edited Loading

Choose a reason for hiding this comment

Voxelot Jan 13, 2023 • edited Loading

Choose a reason for hiding this comment

Voxelot Jan 13, 2023

Choose a reason for hiding this comment

Refactoring of the `graphql-api` service to use ports for `Database` #858

Refactoring of the `graphql-api` service to use ports for `Database` #858

ControlCplusControlV commented Dec 20, 2022 •

edited by xgreenx

Loading

Voxelot Jan 13, 2023 •

edited

Loading

Voxelot Jan 13, 2023 •

edited

Loading