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

feat!: consider message on resources cache #2872

Merged
merged 34 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
9642661
add txID property to global resource cache
Torres-ssf Jul 31, 2024
eaf33c3
implement method delByTxID
Torres-ssf Jul 31, 2024
35ce52a
using TX ID when setting the cache
Torres-ssf Jul 31, 2024
421260c
unsettig cached resources when TX fails or squeezed out
Torres-ssf Jul 31, 2024
cd0aea8
update memory cache test
Torres-ssf Aug 1, 2024
6529f61
add unit test
Torres-ssf Aug 1, 2024
84c5cf8
Merge branch 'master' into st/fix/unset-cached-resource-for-failed-tx
Torres-ssf Aug 1, 2024
6bdbb3b
add test case for memory cache
Torres-ssf Aug 2, 2024
414e603
refact test suites
Torres-ssf Aug 2, 2024
ca102eb
add changeset
Torres-ssf Aug 2, 2024
3413b65
ajust test description
Torres-ssf Aug 2, 2024
ff0dc53
implement ResourceCache class
Torres-ssf Aug 5, 2024
7eeb61f
refact code to use ResourceCache
Torres-ssf Aug 5, 2024
93ccb88
remove old MemoryCache
Torres-ssf Aug 5, 2024
a1c9ac1
add new changeset
Torres-ssf Aug 5, 2024
2751d92
rename cacheUtxo flag to resourceCacheTTL
Torres-ssf Aug 5, 2024
62ab680
Merge branch 'master' into st/fix/unset-cached-resource-for-failed-tx
Torres-ssf Aug 5, 2024
8aca9c2
fix comment
Torres-ssf Aug 5, 2024
09526f0
add method clear to resource cache
Torres-ssf Aug 5, 2024
1a8ab1a
improve docs
Torres-ssf Aug 5, 2024
cc818e6
edit provider options docs
Torres-ssf Aug 5, 2024
7e89b4a
fix constant name
Torres-ssf Aug 5, 2024
b26de3a
Merge branch 'master' into st/fix/unset-cached-resource-for-failed-tx
Torres-ssf Aug 5, 2024
207c73e
Merge branch 'master' into st/fix/unset-cached-resource-for-failed-tx
Torres-ssf Aug 6, 2024
c3d20a8
Merge branch 'master' into st/fix/unset-cached-resource-for-failed-tx
Torres-ssf Aug 6, 2024
9c3eb66
Merge branch 'master' into st/fix/unset-cached-resource-for-failed-tx
Torres-ssf Aug 6, 2024
6ab9f2b
Update packages/account/src/providers/provider.test.ts
Torres-ssf Aug 7, 2024
99e8e3c
Update packages/account/src/providers/provider.test.ts
Torres-ssf Aug 7, 2024
e05c96c
Update packages/account/src/providers/provider.test.ts
Torres-ssf Aug 7, 2024
78c3794
Merge branch 'master' into st/fix/unset-cached-resource-for-failed-tx
Torres-ssf Aug 7, 2024
0ddad8e
Merge branch 'master' into st/fix/unset-cached-resource-for-failed-tx
Torres-ssf Aug 8, 2024
db75955
Update packages/account/src/providers/provider.test.ts
Torres-ssf Aug 8, 2024
318fa60
lintfix
Torres-ssf Aug 8, 2024
6e6ba87
Merge branch 'master' into st/fix/unset-cached-resource-for-failed-tx
Torres-ssf Aug 9, 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/lazy-plants-fry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@fuel-ts/account": patch
---

fix: unset cached resource for failed TX
arboleya marked this conversation as resolved.
Show resolved Hide resolved
63 changes: 49 additions & 14 deletions packages/account/src/providers/memory-cache.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getRandomB256 } from '@fuel-ts/address';
import { randomBytes } from '@fuel-ts/crypto';
import type { BytesLike } from '@fuel-ts/interfaces';
import { hexlify } from '@fuel-ts/utils';
Expand All @@ -9,6 +10,8 @@ import { MemoryCache } from './memory-cache';
* @group browser
*/
describe('Memory Cache', () => {
const txID = getRandomB256();

it('can construct [valid numerical ttl]', () => {
const memCache = new MemoryCache(1000);

Expand Down Expand Up @@ -53,14 +56,14 @@ describe('Memory Cache', () => {
const memCache = new MemoryCache(ttl);
const value = randomBytes(8);

expect(memCache.set(value)).toBeGreaterThanOrEqual(expiresAt);
expect(memCache.set(value, txID)).toBeGreaterThanOrEqual(expiresAt);
});

it('can get [valid key]', () => {
const value = randomBytes(8);
const memCache = new MemoryCache(100);

memCache.set(value);
memCache.set(value, txID);

expect(memCache.get(value)).toEqual(value);
});
Expand All @@ -69,7 +72,7 @@ describe('Memory Cache', () => {
const value = randomBytes(8);
const memCache = new MemoryCache(100);

memCache.set(value);
memCache.set(value, txID);

expect(memCache.get(value)).toEqual(value);
});
Expand All @@ -78,7 +81,7 @@ describe('Memory Cache', () => {
const value = randomBytes(8);
const memCache = new MemoryCache(1);

memCache.set(value);
memCache.set(value, txID);

await new Promise((resolve) => {
setTimeout(resolve, 10);
Expand All @@ -91,7 +94,7 @@ describe('Memory Cache', () => {
const value = randomBytes(8);
const memCache = new MemoryCache(1);

memCache.set(value);
memCache.set(value, txID);

await new Promise((resolve) => {
setTimeout(resolve, 10);
Expand All @@ -104,7 +107,7 @@ describe('Memory Cache', () => {
const value = randomBytes(8);
const memCache = new MemoryCache(100);

memCache.set(value);
memCache.set(value, txID);
memCache.del(value);

expect(memCache.get(value)).toEqual(undefined);
Expand All @@ -118,9 +121,9 @@ describe('Memory Cache', () => {

const memCache = new MemoryCache(100);

memCache.set(value1);
memCache.set(value2);
memCache.set(value3);
memCache.set(value1, txID);
memCache.set(value2, txID);
memCache.set(value3, txID);

expect(memCache.getActiveData()).containSubset(EXPECTED);
});
Expand All @@ -132,11 +135,11 @@ describe('Memory Cache', () => {
const EXPECTED: BytesLike[] = [value1, value2, oldValue];

let memCache = new MemoryCache(500);
memCache.set(value1);
memCache.set(value2);
memCache.set(value1, txID);
memCache.set(value2, txID);

memCache = new MemoryCache(1);
memCache.set(oldValue);
memCache.set(oldValue, txID);

await new Promise((resolve) => {
setTimeout(resolve, 10);
Expand All @@ -151,11 +154,43 @@ describe('Memory Cache', () => {
expect(memCache.getAllData()).containSubset(EXPECTED);
});

it('should delete cached values by its txID', () => {
const value1 = randomBytes(8);
const value2 = randomBytes(8);
const value3 = randomBytes(8);

const ttl = 1000;

const txId1 = 'tx-id-1';
const txId2 = 'tx-id-2';

const memCache1 = new MemoryCache(ttl);
const memCache2 = new MemoryCache(ttl);

memCache1.set(value1, txId1);
memCache1.set(value2, txId2);

memCache2.set(value3, txId1);

expect(memCache1.getActiveData()).toContain(value1);
expect(memCache1.getActiveData()).toContain(value2);

expect(memCache2.getActiveData()).toContain(value3);

memCache1.delByTxID(txId1);

// Value 2 is from TX ID 2, so it should not be deleted
expect(memCache1.getActiveData()).toContain(value2);

expect(memCache1.getActiveData()).not.toContain(value1);
expect(memCache2.getActiveData()).not.toContain(value3);
});

it('should validate that MemoryCache uses a global cache', async () => {
const oldValue = randomBytes(8);

const instance1 = new MemoryCache(1000);
instance1.set(oldValue);
instance1.set(oldValue, txID);

await new Promise((resolve) => {
setTimeout(resolve, 200);
Expand All @@ -164,7 +199,7 @@ describe('Memory Cache', () => {
const newValue = randomBytes(8);

const instance2 = new MemoryCache(100);
instance2.set(newValue);
instance2.set(newValue, txID);

const activeData = instance2.getActiveData();

Expand Down
12 changes: 11 additions & 1 deletion packages/account/src/providers/memory-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ type Cache = {
[key: string]: {
expires: number;
value: BytesLike;
txID: string;
};
};
const cache: Cache = {}; // it's a cache hash ~~> cash?
Expand Down Expand Up @@ -38,12 +39,13 @@ export class MemoryCache {
return undefined;
}

set(value: BytesLike): number {
set(value: BytesLike, txID: string): number {
const expiresAt = Date.now() + this.ttl;
const key = hexlify(value);
cache[key] = {
expires: expiresAt,
value,
txID,
};

return expiresAt;
Expand Down Expand Up @@ -75,4 +77,12 @@ export class MemoryCache {
const key = hexlify(value);
delete cache[key];
}

delByTxID(txID: string) {
Object.keys(cache).forEach((key) => {
if (cache[key].txID === txID) {
delete cache[key];
}
});
}
}
81 changes: 66 additions & 15 deletions packages/account/src/providers/provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -435,12 +435,6 @@ describe('Provider', () => {

const cachedCoins = provider.cache?.getActiveData() || [];
expect(new Set(cachedCoins)).toEqual(new Set(EXPECTED));

// clear cache
const activeData = provider.cache?.getActiveData() || [];
activeData.forEach((coin) => {
provider.cache?.del(coin);
});
});

it('should NOT cache UTXOs when TX submission fails', async () => {
Expand All @@ -462,6 +456,9 @@ describe('Provider', () => {
const maxFee = 100_000;
const transferAmount = 10_000;

const { coins } = await wallet.getCoins(baseAssetId);
const utxoIds = coins.map((c) => c.id);

// No enough funds to pay for the TX fee
const resources = await wallet.getResourcesToSpend([[transferAmount, baseAssetId]]);

Expand All @@ -478,8 +475,62 @@ describe('Provider', () => {
);

// No UTXOs were cached since the TX submission failed
const cachedCoins = provider.cache?.getActiveData() || [];
expect(cachedCoins).lengthOf(0);
utxoIds.forEach((id) => {
expect(provider.cache?.get(id)).toBeUndefined();
});
});

it('should unset cached UTXOs when TX execution fails', async () => {
using launched = await setupTestProviderAndWallets({
nodeOptions: {
args: ['--poa-instant', 'false', '--poa-interval-period', '1s'],
},
walletsConfig: {
coinsPerAsset: 2,
amountPerCoin: 100_000,
},
});
const {
provider,
wallets: [wallet, receiver],
} = launched;

const baseAssetId = provider.getBaseAssetId();
const maxFee = 100_000;
const transferAmount = 10_000;

const { coins } = await wallet.getCoins(baseAssetId);
const utxoIds = coins.map((c) => c.id);

// Should fetch resources enough to pay for the TX fee and transfer amount
const resources = await wallet.getResourcesToSpend([[maxFee + transferAmount, baseAssetId]]);

const request = new ScriptTransactionRequest({
maxFee,
// No enough gas to execute the TX
gasLimit: 0,
});

request.addCoinOutput(receiver.address, transferAmount, baseAssetId);
request.addResources(resources);

// TX submission will succeed
const submitted = await wallet.sendTransaction(request, { estimateTxDependencies: false });

// UTXOs were cached since the TX submission succeeded
utxoIds.forEach((id) => {
expect(provider.cache?.get(id)).toBeDefined();
});

// TX execution will fail
await expectToThrowFuelError(() => submitted.waitForResult(), {
code: ErrorCode.SCRIPT_REVERTED,
});

// Ensure user's UTXOs were unset from the cache
utxoIds.forEach((id) => {
expect(provider.cache?.get(id)).toBeUndefined();
});
});

it('should ensure cached UTXOs are not being queried', async () => {
Expand Down Expand Up @@ -508,14 +559,14 @@ describe('Provider', () => {
// One of the UTXOs will be cached as the TX submission was successful
await wallet.transfer(receiver.address, transferAmount);

const cachedUtxos = provider.cache?.getActiveData() || [];

// Ensure the cached UTXO is the only one in the cache
expect(cachedUtxos.length).toBe(1);
const cachedUtxos = provider.cache?.getActiveData();

// Determine the used UTXO and the unused UTXO
const usedUtxo = cachedUtxos[0];
const unusedUtxos = coins.filter((coin) => coin.id !== usedUtxo);
const usedUtxo = coins.find((coin) => cachedUtxos?.includes(coin.id));
const unusedUtxos = coins.filter((coin) => coin.id !== usedUtxo?.id);

expect(usedUtxo).toBeDefined();
expect(unusedUtxos).toBeDefined();

// Spy on the getCoinsToSpend method to ensure the cached UTXO is not being queried
const resourcesToSpendSpy = vi.spyOn(provider.operations, 'getCoinsToSpend');
Expand All @@ -536,7 +587,7 @@ describe('Provider', () => {
],
excludedIds: {
messages: [],
utxos: [usedUtxo],
utxos: expect.arrayContaining([usedUtxo?.id]),
},
});
});
Expand Down
6 changes: 3 additions & 3 deletions packages/account/src/providers/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -683,14 +683,14 @@ Supported fuel-core version: ${supportedVersion}.`
/**
* @hidden
*/
#cacheInputs(inputs: TransactionRequestInput[]): void {
#cacheInputs(inputs: TransactionRequestInput[], transactionId: string): void {
if (!this.cache) {
return;
}

inputs.forEach((input) => {
if (input.type === InputType.Coin) {
this.cache?.set(input.id);
this.cache?.set(input.id, transactionId);
}
});
}
Expand Down Expand Up @@ -727,7 +727,7 @@ Supported fuel-core version: ${supportedVersion}.`
const {
submit: { id: transactionId },
} = await this.operations.submit({ encodedTransaction });
this.#cacheInputs(transactionRequest.inputs);
this.#cacheInputs(transactionRequest.inputs, transactionId);

return new TransactionResponse(transactionId, this, abis);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ export class TransactionResponse {

for await (const { statusChange } of subscription) {
if (statusChange.type === 'SqueezedOutStatus') {
this.unsetResourceCache();
throw new FuelError(
ErrorCode.TRANSACTION_SQUEEZED_OUT,
`Transaction Squeezed Out with reason: ${statusChange.reason}`
Expand Down Expand Up @@ -278,6 +279,7 @@ export class TransactionResponse {
const { gqlTransaction, receipts } = transactionResult;

if (gqlTransaction.status?.type === 'FailureStatus') {
this.unsetResourceCache();
const { reason } = gqlTransaction.status;
throw extractTxError({
receipts,
Expand Down Expand Up @@ -311,4 +313,8 @@ export class TransactionResponse {
): Promise<TransactionResult<TTransactionType>> {
return this.waitForResult<TTransactionType>(contractsAbiMap);
}

private unsetResourceCache() {
this.provider.cache?.delByTxID(this.id);
}
}
Loading