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

Add CLI command to list transactions for a given deployment #821

Merged
merged 3 commits into from
Oct 24, 2024

Conversation

zoeyTM
Copy link
Contributor

@zoeyTM zoeyTM commented Oct 8, 2024

resolves #805

Screen Shot 2024-10-08 at 4 25 53 AM Screen Shot 2024-10-08 at 4 26 14 AM

@zoeyTM zoeyTM requested review from alcuadrado and kanej October 8, 2024 08:31
Copy link
Member

@alcuadrado alcuadrado left a comment

Choose a reason for hiding this comment

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

This is looking good. I left 3 comments.

@zoeyTM zoeyTM requested a review from alcuadrado October 21, 2024 03:49
@zoeyTM zoeyTM force-pushed the list-transactions branch from b606030 to 745a80c Compare October 21, 2024 03:49
@zoeyTM zoeyTM force-pushed the list-transactions branch from 745a80c to 1fb7c0f Compare October 21, 2024 05:19
);

const networkInteraction =
exState.networkInteractions[message.networkInteractionId - 1];
Copy link
Member

Choose a reason for hiding this comment

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

This -1 caught me by surprise tbh. Do we always have the network interaction ids be 1, 2, 3, ...

TBH I don't remember, and I couldn't find it easily in the code.

Copy link
Member

Choose a reason for hiding this comment

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

I search the code, and we normally do

const onchainInteraction = executionState.networkInteractions.find(
    (interaction) => interaction.id === networkInteractionId
  );

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The -1 is because i'm indexing it directly, not using a find. As far as I know, we've always started the networkInteraction ID's at 1

Copy link
Member

@alcuadrado alcuadrado left a comment

Choose a reason for hiding this comment

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

Left a minor comment, but otherwise LGTM

@zoeyTM zoeyTM merged commit cb37446 into development Oct 24, 2024
10 checks passed
@zoeyTM zoeyTM deleted the list-transactions branch October 24, 2024 07:38
@zoeyTM zoeyTM mentioned this pull request Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add a way of visualizing transaction history
2 participants