-
Notifications
You must be signed in to change notification settings - Fork 4
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
fix multisignature spec file error #19
Conversation
Multisignature is still encoded through Amino and prefix is added when encoded to Amino pubkey, but the prefix was not modified for ostracon, so multisig pubkey was not encoded correctly. Accordingly multisig address was not generated correctly. The problem was solved by modifying the prefix to match the ostracon.
After change Amino prefix to fit with Ostracon, test codes crashed because every addresses and pubkeys in test code were made for former Amino prefix. Change addresses and pubkeys to Ostracon version.
generate new enigma bech32 pubkey from Ostracon secp256k1 pubkey
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.
Please remove the comment lines of the old modified code.
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
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
signingInstruction.msgs, | ||
signingInstruction.fee, | ||
signingInstruction.memo, | ||
// signingInstruction.sigBlockHeight, |
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.
How about removing the sigBlockHeight instead of commenting it out?
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 didn't know what sigBlockHeight was for, so I left it alone. I will remove it later.
[address3, signature3], | ||
[address4, signature4], | ||
]), | ||
// signingInstruction.sigBlockHeight, |
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.
Same comment with line 238.
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.
Same in here~
// On the composer's machine signing instructions are created. | ||
// The composer does not need to be one of the signers. | ||
const signingInstruction = await (async () => { | ||
const client = await StargateClient.connect(simapp.tendermintUrl); |
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.
How about changing tendermint to ostracon?
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.
The name tendermint is being used in quite a few places, so I think it would be good to get rid of it at once later.
Multisignature is still encoded through Amino and prefix is added when encoded to Amino pubkey, but the prefix was not modified for ostracon, so multisig pubkey was not encoded correctly.
Accordingly multisig address was not generated correctly.
The problem was solved by modifying the prefix to match the ostracon.