-
Notifications
You must be signed in to change notification settings - Fork 26
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 function to encode a function call #761
Conversation
Looking at our other examples I don't know why they refer to `named library`, I am going to leave them and quietly shut the door behind me.
packages/core/src/internal/validation/futures/validateNamedEncodeFunctionCall.ts
Show resolved
Hide resolved
packages/core/test/reconciliation/futures/reconcileNamedEncodeFunctionCall.ts
Outdated
Show resolved
Hide resolved
packages/core/test/reconciliation/futures/reconcileNamedEncodeFunctionCall.ts
Outdated
Show resolved
Hide resolved
}); | ||
}); | ||
|
||
it("should not validate a non-existant hardhat contract", async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this not testing the m.contract
validation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this validation (as well as the normal function call) has its own version of the check that this is testing.
That being said, this comment made me notice that I somehow forgot to swap the validation functions when I copied these tests from the normal function call tests, so I'll push a commit with that change.
packages/core/src/internal/execution/future-processor/helpers/future-resolvers.ts
Outdated
Show resolved
Hide resolved
packages/core/src/internal/reconciliation/helpers/reconcile-data.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few comments and nit picks. The only real question is around the id and default id.
Otherwise this is really solid work. Nicely done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolves #760