-
Notifications
You must be signed in to change notification settings - Fork 336
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
Missing Active Authority while adding active authority #630
Comments
Found something interesting: the transaction generated by GUI is rejected by witness_node, because witness_node think it's signed by other keys but not the active key and owner key of the account. The transaction is:
witness_node think the transaction is signed by
Chain-id of testnet is Either something wrong while signing the transaction, or something wrong while verifying the signatures. Thoughts? |
Same transaction can be signed in CLI wallet and got accepted by witness_node without issue. So it's perhaps a GUI issue. |
In the stealth branch, I can find testaccount1 and add that public key. In the transaction preview the TEST5... key comes before the TEST8... key (unlike what you see above in the " transaction generated by GUI is rejected by witness_node,"). The same code is in master, but I'm having a hard time starting the master node to confirm, due to a missing loader. |
Here is the fix... It looks like addresses and public keys should be sorted using descending order unlike most other types (which would be sorted ascending). |
Unfortunately there is no explicit
It's terrible for 3rd-party clients, including Graphene-ui. The patch provided by @jcalfee in above comment won't work in all cases. We need a hard fork to clearly define the sorting mechanism of public keys. @theoreticalbts thoughts? By the way, according to the code, addresses are sorted by the binary representation, so likely same to base58 string sorting. A result of address sorting test is here:
So GUI code which hasn't applied above patch should work with addresses. However, address_authority is implemented for backward compatibility only, probably we'll disable it in the future. |
Yeah, the issue here is that we have
which means the |
The currently used sorting mechanism is to convert the keys to addresses, then sort by address. Addresses themselves are sorted by hex value. The above branch (updated below with a better commit message) implements a |
OK, at least we don't need to hard fork. @theoreticalbts may I ask why (where is the code) public keys are sorting in this way? I'm going to close this ticket and move discussion to graphene-ui repo. |
It's not done explicitly in the code, rather since Figuring out that was what the code is doing is non-trivial. I wrote the unit test above (it's not really a test, it just creates two keys and compares them). Then set a breakpoint at the line that compares the two keys and stepped into it with GDB. We should probably implement the ordering as a function |
Their are 5 different address formats... I think I know which one to use though, we have a default one. I'll look at the code and maybe I can use your unit test to confirm. |
Fixed in GUI 2.0.160406 |
Merge release to master
Exception
Missing Active Authority
is reported while trying to addTEST56ankGHKf6qUsQe7vPsXTSEqST6Dt1ff73aV3YQbedzRua8NLQ
as another active key authority to http://testnet.bitshares.eu/#/account/testaccount1/permissions/, even if threshold set to 1 and weight of all keys set to 1. Private key of the new new public key is5K9piFpyJ15zcYBc4R3gAeLFYb6PuRFEiDCbzLkSL9C4d7fWLyB
.Original key pairs of that account:
TEST8me6d9PqzTgcoHxx6b4rnvWVTqz11kafidRAZwfacJkcJtfd75
5JBqM3timYgxvAjd7ddBhr4mdQq8foP3rLVXrphZCs6aYdJ48u2
TEST6vGWLjXrB971RXKoejCsZoAsv1wbfEmwNK7deN3kcdYQFck8wb
5JSKnvw8AodxNrgE8uETCApJUmZ7u3t1tSWp3ay823MNhmyAmqS
The account can vote without issue (no change to owner or active authorities), so perhaps an issue while validating the new key.
The text was updated successfully, but these errors were encountered: