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.

Sign up for GitHub

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

Jump to bottom

chore: optimize getTransactions query #3336

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
+54 −7
Draft

chore: optimize getTransactions query #3336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
172e7a6
rename graphql fragment
Torres-ssf Oct 15, 2024
6be245d
put fragment definition on proper place
Torres-ssf Oct 15, 2024
3bc88bb
use transactionRawPayloadFragment for getTransactions query
Torres-ssf Oct 15, 2024
a5f6343
add test for getTransactions
Torres-ssf Oct 15, 2024
3b3f04b
fix test case name
Torres-ssf Oct 15, 2024
36a7208
improve test
Torres-ssf Oct 15, 2024
b1f5f67
add test to ensure getBlockWithTransactions does not fetch excessive …
Torres-ssf Oct 15, 2024
c78aeb4
add changeset
Torres-ssf Oct 15, 2024
78119c6
slim down query even more
Torres-ssf Oct 15, 2024
6de1168
adjust test case
Torres-ssf Oct 15, 2024
fff1af4
rename vars
Torres-ssf Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/nervous-impalas-serve.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@fuel-ts/account": patch
Copy link
Member

Choose a reason for hiding this comment

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

"@fuel-ts/account": minor

Isn't this a breaking change given we changing the response of getTransactions to return two less fields? i.e. the id and transactionStatusFragment ?

Copy link
Contributor Author

@Torres-ssf Torres-ssf Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

But @maschad The Provider.getTransactions has exactly the same response, no property was removed from it.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with you but if someone called provider.operations.getTransactions() the response would change and thus it's breaking.

Copy link
Contributor

Choose a reason for hiding this comment

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

We made #3286 and #3296 breaking for the same reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maschad Oook I get you now.

I think this one went unnoticed #3309

---

chore: optimize `getTransactions` query
14 changes: 7 additions & 7 deletions packages/account/src/providers/operations.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ fragment transactionFragment on Transaction {
}
}

fragment transactionRawPayloadFragment on Transaction {
id
rawPayload
}

fragment inputEstimatePredicatesFragment on Input {
... on InputCoin {
predicateGasUsed
Expand Down Expand Up @@ -487,7 +492,7 @@ query getTransactions(
transactions(after: $after, before: $before, first: $first, last: $last) {
edges {
node {
...transactionFragment
rawPayload
}
}
pageInfo {
Expand Down Expand Up @@ -553,7 +558,7 @@ query getBlockWithTransactions($blockId: BlockId, $blockHeight: U32) {
block(id: $blockId, height: $blockHeight) {
...blockFragment
transactions {
...transactionRawPayload
...transactionRawPayloadFragment
}
}
}
Expand All @@ -571,11 +576,6 @@ query getBlocks($after: String, $before: String, $first: Int, $last: Int) {
}
}

fragment transactionRawPayload on Transaction {
id
rawPayload
}

query getCoin($coinId: UtxoId!) {
coin(utxoId: $coinId) {
...coinFragment
Expand Down
42 changes: 42 additions & 0 deletions packages/account/src/providers/provider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1872,6 +1872,48 @@ Supported fuel-core version: ${mock.supportedVersion}.`
});
});

it('ensures getTransactions does not fetch unused data', async () => {
using launched = await setupTestProviderAndWallets();
const { provider } = launched;

await provider.produceBlocks(1);

const { transactions } = await provider.operations.getTransactions({
first: 1,
});

expect(transactions.edges.length).toBe(1);

const expectedData = {
rawPayload: expect.any(String),
};

expect(transactions.edges[0].node).toStrictEqual(expectedData);
});

it('ensures getBlockWithTransactions does not fetch unused transaction data', async () => {
using launched = await setupTestProviderAndWallets();
const { provider } = launched;

await provider.produceBlocks(1);

const blockNumber = await provider.getBlockNumber();

const { block } = await provider.operations.getBlockWithTransactions({
blockHeight: blockNumber.toString(),
});

expect(block).toBeDefined();
expect(block?.transactions.length).toBe(1);

const expectedData = {
id: expect.any(String),
rawPayload: expect.any(String),
};

expect(block?.transactions?.[0]).toStrictEqual(expectedData);
});

describe('paginated methods', () => {
test('can properly use getCoins', async () => {
const totalCoins = RESOURCES_PAGE_SIZE_LIMIT + 1;
Expand Down