-
Notifications
You must be signed in to change notification settings - Fork 123
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
Optimism support #153
Optimism support #153
Conversation
@@ -183,7 +183,7 @@ func (tx *Tx) Sign(signatures []pack.Bytes65, pubKey pack.Bytes) error { | |||
continue | |||
} | |||
} else { | |||
if txscript.IsPayToWitnessScriptHash(sigScript) || txscript.IsPayToWitnessScriptHash(sigScript) { | |||
if txscript.IsPayToWitnessPubKeyHash(sigScript) || txscript.IsPayToWitnessScriptHash(sigScript) { |
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 probably don't have the context but was our old bitcoin implementation broken?
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.
It didn't affect our internal usage, but may have affected external users since that particular branch wasn't being handled.
case []pack.U256: | ||
v := make([]*big.Int, len(val)) | ||
for i := range val { | ||
elem, ok := big.NewInt(0).SetString(val[i].String(), 10) | ||
if !ok { | ||
panic(fmt.Errorf("encoding list item: %v", err)) | ||
} | ||
v[i] = elem | ||
} | ||
ethval = v | ||
ty, err = abi.NewType("uint256[]", "", nil) | ||
case []Address: | ||
ethval = val | ||
ty, err = abi.NewType("address[]", "", nil) | ||
|
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.
are these optimism specific or did we never face such type of params before?
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.
Nope, these are specific to some new slashing contracts being built. We should add support for even more types when we can!
No description provided.