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

feat(store-sync): add status and block number to return type of waitForTransaction #2668

Merged
merged 4 commits into from
Apr 16, 2024
Merged

feat(store-sync): add status and block number to return type of waitForTransaction #2668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0cfdda3
feat(store-sync): add status and block number to return type of waitF…
alvrs Apr 16, 2024
17ec3d5
Create warm-flies-reply.md
alvrs Apr 16, 2024
f10d344
Update warm-flies-reply.md
alvrs Apr 16, 2024
4e35c97
add tx hash to return type
alvrs Apr 16, 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/warm-flies-reply.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@latticexyz/store-sync": patch
---

`waitForTransaction` now returns a `Promise<{ blockNumber: bigint, status: "success" | "reverted" }>` instead of `Promise<void>`, to allow consumers to react to reverted transactions without refetching the transaction receipt.
6 changes: 4 additions & 2 deletions packages/store-sync/src/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Address, Block, Hex, Log, PublicClient } from "viem";
import { Address, Block, Hex, Log, PublicClient, TransactionReceipt } from "viem";
import { StoreEventsAbiItem, StoreEventsAbi } from "@latticexyz/store";
import { resolveConfig } from "@latticexyz/store/internal";
import { Observable } from "rxjs";
Expand Down Expand Up @@ -113,11 +113,13 @@ export type SyncOptions<config extends StoreConfig = StoreConfig> = {
};
};

export type WaitForTransactionResult = Pick<TransactionReceipt, "blockNumber" | "status" | "transactionHash">;

export type SyncResult = {
latestBlock$: Observable<Block>;
latestBlockNumber$: Observable<bigint>;
storedBlockLogs$: Observable<StorageAdapterBlock>;
waitForTransaction: (tx: Hex) => Promise<void>;
waitForTransaction: (tx: Hex) => Promise<WaitForTransactionResult>;
};

// TODO: add optional, original log to this?
Expand Down
22 changes: 15 additions & 7 deletions packages/store-sync/src/createStoreSync.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
SyncOptions,
SyncResult,
internalTableIds,
WaitForTransactionResult,
} from "./common";
import { createBlockStream } from "@latticexyz/block-logs-stream";
import {
Expand Down Expand Up @@ -262,7 +263,7 @@ export async function createStoreSync<config extends StoreConfig = StoreConfig>(
);

// TODO: move to its own file so we can test it, have its own debug instance, etc.
async function waitForTransaction(tx: Hex): Promise<void> {
async function waitForTransaction(tx: Hex): Promise<WaitForTransactionResult> {
debug("waiting for tx", tx);

// This currently blocks for async call on each block processed
Expand All @@ -271,25 +272,32 @@ export async function createStoreSync<config extends StoreConfig = StoreConfig>(
// We use `mergeMap` instead of `concatMap` here to send the fetch request immediately when a new block range appears,
// instead of sending the next request only when the previous one completed.
mergeMap(async (blocks) => {
const txs = blocks.flatMap((block) => block.logs.map((op) => op.transactionHash).filter(isDefined));
if (txs.includes(tx)) return true;
for (const block of blocks) {
const txs = block.logs.map((op) => op.transactionHash);
// If the transaction caused a log, it must have succeeded
if (txs.includes(tx)) {
return { blockNumber: block.blockNumber, status: "success" as const, transactionHash: tx };
}
}

try {
const lastBlock = blocks[0];
debug("fetching tx receipt for block", lastBlock.blockNumber);
const receipt = await publicClient.getTransactionReceipt({ hash: tx });
return lastBlock.blockNumber >= receipt.blockNumber;
const { status, blockNumber, transactionHash } = await publicClient.getTransactionReceipt({ hash: tx });
if (lastBlock.blockNumber >= blockNumber) {
Copy link
Member

Choose a reason for hiding this comment

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

looking at this now, I wonder if this is maybe the cause of infinite spinning sometimes

if we get a receipt, we only sometimes return it?

I think this might be here to make sure the syncing is caught up to where the block for this receipt came through, but just want to check/confirm that we would refetch this transaction receipt while waiting, or if this would cause it to sort of "fall out" of the loop and ~wait indefinitely

Copy link
Member Author

Choose a reason for hiding this comment

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

We would refetch it when processing the next block. Agree we should add caching to fetching tx receipts though.

Copy link
Member Author

Choose a reason for hiding this comment

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

#2670

return { status, blockNumber, transactionHash };
}
} catch (error) {
if (error instanceof TransactionReceiptNotFoundError) {
return false;
return;
}
throw error;
}
}),
tap((result) => debug("has tx?", tx, result)),
);

await firstValueFrom(hasTransaction$.pipe(filter(identity)));
return await firstValueFrom(hasTransaction$.pipe(filter(isDefined)));
}

return {
Expand Down
Loading