Skip to content
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 #864 sign_builder_transaction sometimes doesn't work #865

Merged
merged 1 commit into from
Apr 21, 2018

Conversation

pmconrad
Copy link
Contributor

sign_transaction calls get_required_signatures to find out which signatures need to be added. get_required_signatures will not report signatures that are already present. Later, however, sign_transaction removes the signatures that were already there, and only adds those reported by get_required_signatures.

The fix is to clear all signatures before calling get_required_signatures.

Resolves #864

@abitmore
Copy link
Member

Good catch.

Actually I think sign_transaction should not remove old signatures when adding new signatures, then it will be more useful for multi-signing. By the way, if modified expiration and / or TaPoS fileds, the old signatures will become invalid. So I think it's better to check those fields as well, and try to not change them. Or perhaps add another command for this feature.

@abitmore abitmore added this to the 201805 - Non-Consensus-Changing Release milestone Apr 20, 2018
@pmconrad
Copy link
Contributor Author

Yes, that might be a useful use-case. We shouldn't change the existing API call for this though. Better add a new one.

@abitmore abitmore merged commit 9e78c2d into bitshares:develop Apr 21, 2018
@abitmore abitmore changed the title Fix #864 Fix #864 sign_builder_transaction sometimes doesn't work Apr 21, 2018
@pmconrad pmconrad deleted the 864_fix_sign_transaction branch June 22, 2018 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants