-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
graphql-api
ports for database queries
#890
graphql-api
ports for database queries
#890
Conversation
Added `Transactional` trait with `transaction` method. Used `BlockId` instead of `Bytes32` in `Database`. Created ports for `block.rs`, `tx.rs`, `message.rs` and updated corresponding schema. Added and used in ports `BoxedIter` instead of associated types(In most cases they were `Box<dyn>` too).
Co-authored-by: Green Baneling <XgreenX9999@gmail.com>
@@ -3,7 +3,7 @@ use crate::{ | |||
state::in_memory::transaction::MemoryTransactionView, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should the name of this file/module be renamed now that we no longer use Transactional
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will update it in a separate PR to not lose history
@@ -1,10 +1,10 @@ | |||
use crate::{ | |||
database::resource::{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should make the whole schema module a sub-module of the new graphql service module.
e.g.
src/schema.rs
-> src/grapqhl_api/schema.rs
src/schema/*
-> src/graphql_api/schema/*
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, of course, we do it step by step)
crates/storage/src/transactional.rs
Outdated
/// The type is transactional and holds uncommitted state. | ||
pub trait Transactional<Storage>: AsRef<Storage> + AsMut<Storage> + Send + Sync { | ||
/// The types is transactional and may create `StorageTransaction`. | ||
pub trait Transactional<Storage> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to be used anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be, it is not final PR=)
balances.reverse(); | ||
} | ||
|
||
let balances = balances.into_iter(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not updated yet=)
…p.rs` uses it right now for `VMDatabase`. Introduced `CompressedCoin` and `Coin`(contains `UtxoId`). `CompressedCoin` is used during work with database, `Coin` in external API. Moved resources into `fuel-core-type` and simplified it by removing generics. Tried to re-work `dapp.rs` but the change is big, so will do it in a separate PR. Updated `Transactional` to support `Transactional<dyn Database>`
@@ -3,7 +3,7 @@ use crate::{ | |||
state::in_memory::transaction::MemoryTransactionView, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will update it in a separate PR to not lose history
.transaction_status(&transaction_id) | ||
.into_api_result::<TransactionStatus, StorageError>()? | ||
{ | ||
Some(TransactionStatus::Success { block_id, .. }) => block_id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the logic here, not allowing generate proofs for failed transactions.
@@ -200,80 +158,6 @@ impl MessageProof { | |||
} | |||
} | |||
|
|||
struct MessageProofContext<'a>(&'a Database); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to message_proof.rs
graphql-api
ports for blocks, txs, and messagesgraphql-api
ports for database queries
Comments addressed except for issues which @xgreenx has deferred to future PRs, merging upstream |
Transactional
intoTransaction
.Transactional
trait withtransaction
method.BlockId
instead ofBytes32
inDatabase
.block.rs
,tx.rs
,message.rs
,balance.rs
,coin.rs
and updated corresponding schema.BoxedIter
instead of associated types(In most cases they wereBox<dyn>
too).dap.rs
uses it right now forVMDatabase
.CompressedCoin
andCoin
(containsUtxoId
).CompressedCoin
is used during work with the database,Coin
in external API.fuel-core-type
and simplified it by removing generics.dap.rs
but the change is big and also related tofuel-core-executor
, so will do it in a separate PR.Transactional
to supportTransactional<dyn Database>