-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Static link token #15546
Static link token #15546
Conversation
Flakeguard Summary
Found Flaky Tests ❌
|
Flakeguard Summary
Found Flaky Tests ❌
|
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.
Generic suggestion about wrapping the error with address and chain details whenever possible
deployment/address_book.go
Outdated
if wantType == haveType { | ||
counts[wantType]++ | ||
if counts[wantType] > 1 { | ||
return false, fmt.Errorf("found more than one instance of a contract") |
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.
can we show the type in error message?
Wdyt about using multierror and showing all types which will have more than one instance?
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.
type for sure. multierr maybe bit overkill
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.
LGTM!
// and has a distinct ABI from the new LinkToken. | ||
// Corresponds to the ABI | ||
// https://github.com/smartcontractkit/chainlink/blob/develop/core/gethwrappers/generated/link_token_interface/link_token_interface.go#L34 | ||
StaticLinkToken deployment.ContractType = "StaticLinkToken" |
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.
maybe LegacyLinkToken?
Supporting viewing non-B/M link token. Side quest backfilling some tests and improving input validation.