-
Notifications
You must be signed in to change notification settings - Fork 32
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
Remove zkapp_input
#23
Conversation
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.
This is awesome! Thanks for the PR. It looks like you need to run cargo fmt before merging this : o
also might be a good idea to update the documentation in docs/ with the removed field
.iter() | ||
.find(|tx| tx.previous_output.txid == self.zkapp_tx.txid()) | ||
.context("the transaction ID that was passed in the request does not exist")? | ||
.previous_output; |
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.
might be slightly cleaner
fn zkapp_outpoint(&self) -> Result<OutPoint> {
self
.tx
.input
.iter()
.find(|tx| tx.previous_output.txid == self.zkapp_tx.txid())
.context("the transaction ID that was passed in the request does not exist")
.and_then(|input| input.previous_output)
}
Ok(txin.previous_output) | ||
.iter() | ||
.find(|tx| tx.previous_output.txid == self.zkapp_tx.txid()) | ||
.context("the transaction ID that was passed in the request does not exist")? |
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.
maybe a better error here would be "the zkapp transaction ID could not be found in the transaction's inputs"
ensure!( | ||
zkapp_inputs.len() == 1, | ||
"internal error: the transaction does not contain the zkapp being used or it contains duplicate inputs" | ||
); |
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.
nice check
src/committee/orchestrator.rs
Outdated
.get_mut(bob_request.zkapp_input) | ||
.context("couldn't find zkapp input in transaction")? | ||
.witness = witness; | ||
let transaction = bob_request.zkapp_tx_with_witness(witness)?; |
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.
note that we could return the witness to Bob (shorter response) and Bob could fit it in the transaction by himself
The index of the input that contains the
zkapp
being used is part of theunlock_funds
RPC request. However, this is redundant since we can infer it from the unlock transaction inputs using the following equality check:tx.previous_output.txid == self.zkapp_tx.txid()
Fixes #6