fuel-core icon indicating copy to clipboard operation
fuel-core copied to clipboard

Add GraphQL Account type

Open AlicanC opened this issue 3 years ago • 6 comments

This PR basically tries to do two things:

  • Add Account type
  • Change fields that represent an Account (like Block.producer) from HexString256 to Account

The Account type can be extended with fields like balances and transactions so this:

query {
  balances(address: $address) { ... }
  transactions(address: $address) { ... }
}

can become this:

query {
  account(address: $address) {
    balances { ... }
    transactions { ... }
  }
}

The change from HexString256 to Account is so we can pack requirements of a UI into one query:

// Before
let block = await q(`query {
  block(height: ${height}) {
    producer
  }
}`);

let producer = await q(`query {
  account(id: ${block.producer}) {
    address
    producedBlocks { count }
  }
}`);

console.log(`${height} produced by ${producer.address} who produced ${producer.producedBlocks.count} so far!`)

// After
let block = await q(`query {
  block(height: ${height}) {
    producer {
      address
      producedBlocks { count }
    }
  }
}`);

let producer = block.producer;

console.log(`${height} produced by ${producer.address} who produced ${producer.producedBlocks.count} so far!`)

Given the green light, I will:

  • [x] Change HexString256 to Account in all fields necessary
  • [x] Add Account.transactions
  • [x] Add fuel-client implementation and tests

AlicanC avatar Feb 12 '22 00:02 AlicanC

@Voxelot what do you think, should I proceed? Any comments?

AlicanC avatar Feb 12 '22 00:02 AlicanC

Yes this is a much cleaner approach. Eventually, I would like to avoid exposing any of the graphql schema types directly via fuel-gql-client, and only use the proper corresponding fuel-types or other types that are more rust friendly.

Voxelot avatar Feb 12 '22 01:02 Voxelot

@Voxelot ready for review.

One note is that I did not use Account.transactions in fuel-gql-client since Query.transactionsByOwner already does the trick. (Also not planning to use it on fuels-ts, but only on Explorer/Wallet.) For that reason I wrote the test for it in fuel-core/src/schema/account.rs and not fuel-client/src/client/schema/account.rs.

Another note is that there's a lot of code duplication between Account.transactions and Query.transactionsByOwner. I believe a huge amount of it would be solved if we abstracted our "gql args -> db iterator -> gql connection" logic. I need to see more connections before I feel comfortable doing that so I left it.

AlicanC avatar Feb 13 '22 13:02 AlicanC

Bikeshed time: instead of Account, which @Voxelot has pointed out might be confused with the notion of an Ethereum account, we could use a different word like Statement (like a bank statement)?

adlerjohn avatar Feb 28 '22 18:02 adlerjohn

@AlicanC @Voxelot is this pull request obsolete?

rakita avatar Jul 04 '22 14:07 rakita

@rakita the problem mentioned in the OP still exists.

Looks like Sway now has Identity which we could use instead of Account: https://github.com/FuelLabs/sway/blob/master/sway-lib-std/src/identity.sw

AlicanC avatar Jul 04 '22 18:07 AlicanC

closing since this is falling really out of date and we don't really need this abstraction

Voxelot avatar Sep 15 '22 21:09 Voxelot