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

chore: avoid reassigning inputs and outputs at BaseInvocationScope #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hbrooks
Copy link
Member

@hbrooks hbrooks commented Feb 27, 2024

Ellipsis 🚀 This PR description was created by Ellipsis for commit 38abe2b.

Summary:

This PR enhances the Provider and BaseTransactionRequest classes to return additional data from certain functions, and updates BaseInvocationScope to avoid reassigning inputs and outputs, with corresponding test updates.

Key points:

  • Updated estimateTxDependencies and getTransactionCost in Provider class to return outputVariables and missingContractIds.
  • Modified addCoinInput and addMessageInput in BaseTransactionRequest to handle new return values.
  • Updated BaseInvocationScope to avoid reassigning inputs and outputs.
  • Updated test files to reflect these changes.

Generated with ❤️ by ellipsis.dev

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me!

  • Reviewed the entire pull request up to 38abe2b
  • Looked at 254 lines of code in 6 files
  • Took 1 minute and 7 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 1 additional comments because they didn't meet confidence threshold of 50%.
1. /packages/program/src/functions/base-invocation-scope.ts:231:
  • Assessed confidence : 50%
  • Comment:
    The changes in this file seem to be related to the addition of outputVariables and missingContractIds to the return of estimateTxDependencies and getTransactionCost. The author also seems to have made changes to avoid reassigning inputs and outputs from the estimated TX at BaseInvocationScope. The changes seem to be logically correct and I don't see any clear violations of best practices. However, I would need to explore the codebase to understand the context and implications of these changes better.
  • Reasoning:
    The changes in this file seem to be related to the addition of outputVariables and missingContractIds to the return of estimateTxDependencies and getTransactionCost. The author also seems to have made changes to avoid reassigning inputs and outputs from the estimated TX at BaseInvocationScope. The changes seem to be logically correct and I don't see any clear violations of best practices. However, I would need to explore the codebase to understand the context and implications of these changes better.

Workflow ID: wflow_NyRBYfdc5C7xZfBE


Not what you expected? You can customize the content of the reviews using rules. Learn more here.

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.

1 participant