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

Leverage wormscan client on sourcetx middleware #214

Merged
merged 7 commits into from
Oct 27, 2023
Merged

Conversation

mat1asm
Copy link
Collaborator

@mat1asm mat1asm commented Oct 17, 2023

No description provided.

@mat1asm mat1asm changed the base branch from missed-vaas-list-look-ahead-prepare to main October 17, 2023 19:33
Copy link
Collaborator

@abhidtu2014 abhidtu2014 left a comment

Choose a reason for hiding this comment

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

Added some comments, PTAL

await sleep((attempt + 1) * 200); // linear wait
}
} while (++attempt < retries && !txHash);
let txHash = response.data?.txHash || "";
Copy link
Collaborator

Choose a reason for hiding this comment

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

May be we can throw an error if we don't find txHash on the response data object and remove empty string as fallback.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should put a comment somewhere (or ideally on documentation if we had any), but...
It's a design decision not to throw an error. This is becase you wan't to leave this decision to the user of relayer-engine. The source tx hash is strictly necessary for processing some VAAs, but not for others, so it's up to the user to decide.
At some point we thought that maybe we can make this a configuration of the middleware but never got to implement it... for the time been I'd leave it as is

relayer/rpc/wormscan-client.ts Outdated Show resolved Hide resolved
@solanoepalacio
Copy link
Collaborator

Appreciate that you went the extra mile and implemented this 🖤

@mat1asm mat1asm force-pushed the change-sourcetx-client branch from 90a89f1 to 5a44a0f Compare October 20, 2023 14:04
@mat1asm mat1asm force-pushed the change-sourcetx-client branch from 5a44a0f to baa27b3 Compare October 20, 2023 14:04
@mat1asm mat1asm requested review from abhidtu2014 and gabzim October 23, 2023 21:36
@mat1asm mat1asm merged commit 7f97bcc into main Oct 27, 2023
2 checks passed
@mat1asm mat1asm deleted the change-sourcetx-client branch October 27, 2023 18:08
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.

5 participants