-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat!: resolve witness indexes automatically #1069
Conversation
Regarding tests, we could include a case with multiple inputs coming from different wallets for a simple transfer and one for a create transaction |
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.
Love it. Left suggestions around macros.
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 will turn this PR into a draft. Even though I implemented the requested feature, we still have some use-cases with witnesses we need to cover. I will soon make a new version and then we can discuss it. |
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 left some questions.
Nice addition to the UX 👍
2718b1f
closes: #965 BREAKING CHANGE: - `send_transaction`, `dry_run`, `dry_run_no_validation` and `estimate_transaction_cost` need ownership of the `tx`. - removed witness index from `Input` and corresponding functions - wallet's `sign_transaction` now accepts a `TransactionBuilder` Co-authored-by: MujkicA <ahmedmujkic2@gmail.com>
closes: #965
Previously, when spending signed resource, the user had to manually set
witness
indexes. This meant that the user had to pay attention on the signing order and make sure to offset the index if some witnesses where already set.This PR automates the whole process.
Proposed workflow:
sign_transaction
now accepts aTransactionBuilder
TransactionBuilder
the intention is saved inUnresolvedSignatures
append_witness
- this allows the user to add custom witnesses and it will not affect theTxId
Additionally, building a transaction will fail if there are signed resources that are missing corresponding signatures.
BREAKING CHANGE:
send_transaction
,dry_run
,dry_run_no_validation
andestimate_transaction_cost
need ownership of thetx
.Input
and corresponding functionssign_transaction
now accepts aTransactionBuilder
Checklist