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

Fetching block details from database #58

Merged
merged 5 commits into from
Aug 2, 2024

Conversation

PoulavBhowmick03
Copy link

@PoulavBhowmick03 PoulavBhowmick03 commented Aug 2, 2024

Related Issues

Closes #53

Changes

  • Created a new helper function fetchBlockDetails in components/pages/block/fetch-block-details.ts
  • Updated components/pages/block/index.tsx to use the new fetchBlockDetails function
  • Added logs to show in the terminal, what is used to fetch data, JsonRPC or from database

Implementation Details

  • The new fetchBlockDetails function is data source agnostic
  • Two separate functions are implemented:
    • fetchBlockDetailsFromJsonRpc: fetches data from JSON-RPC (existing behavior)
    • fetchBlockDetailsFromDatabase: fetches data from the database (new implementation)
  • The choice between these functions is determined by the presence of the DATABASE_URL environment variable
  • The implementation adheres to the existing design pattern used in the transaction details page

Validation

  • The function fetches the same data whether calling the RPC directly or querying the database
  • It maintains consistency with the existing design pattern implemented in the tx details page

Additional Notes

  • This change requires running op-indexer as a background task alongside the explorer to maintain an up-to-date database mirroring the JSON-RPC endpoint

Copy link

vercel bot commented Aug 2, 2024

Someone is attempting to deploy a commit to the Walnut Team on Vercel.

A member of the Team first needs to authorize it.

@PoulavBhowmick03
Copy link
Author

@saimeunt Do have a look at this!

Copy link
Collaborator

@saimeunt saimeunt left a comment

Choose a reason for hiding this comment

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

@PoulavBhowmick03 nice work, please make the requested changes and we'll merge this ASAP.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This file is a leftover and is no longer needed, please delete it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please only commit the renaming of POSTGRES_URL to DATABASE_URL but keep the default option to use sqlite.

import { prisma } from "@/lib/prisma";

type FetchBlockDetailsReturnType = {
block: Block | null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since these functions returns only a Block or null, there's no need for this helper type, just specify their return type as Block | null directly.

const fetchBlockDetailsFromDatabase = async (
number: bigint,
): Promise<FetchBlockDetailsReturnType> => {
console.log(`Attempting to fetch block ${number} from database`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove all the console.logs, we don't want them outside of testing purpose.

): Promise<FetchBlockDetailsReturnType> => {
console.log(`Attempting to fetch block ${number} from database`);
const block = await prisma.block.findUnique({
where: { number: BigInt(number) },
Copy link
Collaborator

Choose a reason for hiding this comment

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

number is already a bigint, there's no need to cast it to BigInt.

console.log(`Fetching block ${number} from JSON-RPC`);
const block = await l2PublicClient.getBlock({ blockNumber: number });

if (!block) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The next few lines can be simplified by return block ? fromViemBlock(block) : null;

@PoulavBhowmick03
Copy link
Author

@saimeunt , thank you! made the changes, do have a look

saimeunt
saimeunt previously approved these changes Aug 2, 2024
@saimeunt
Copy link
Collaborator

saimeunt commented Aug 2, 2024

@PoulavBhowmick03 there's a final change to be made here:
https://github.com/walnuthq/op-scan/pull/58/files/79e443ffcacf19f3dd28e1ec535a569116cf2eaa#diff-5f601c5ece3a3de8bbb7cb5e0e09bb77dae1d8e04549a987d1d017702cebea2fR8

Remove the curly braces because we're no longer returning an enclosing object.

@PoulavBhowmick03
Copy link
Author

@saimeunt done

@saimeunt saimeunt merged commit 34919b1 into walnuthq:main Aug 2, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fetch block details from database
2 participants