-
Notifications
You must be signed in to change notification settings - Fork 134
[RPCSAGA] verifyutxochaintipinclusionproof rpc #1085
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
fb2cc28
73c8ad0
f045293
6dd8693
0d995fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -941,9 +941,6 @@ impl<PersistedState: ChainStore> ChainState<PersistedState> { | |
| None => Ok(true), | ||
| } | ||
| } | ||
| pub fn acc(&self) -> Stump { | ||
| read_lock!(self).acc.to_owned() | ||
| } | ||
| /// Returns the next required work for the next block, usually it's just the last block's target | ||
| /// but if we are in a retarget period, it's calculated from the last 2016 blocks. | ||
| fn get_next_required_work( | ||
|
|
@@ -1042,8 +1039,18 @@ impl<PersistedState: ChainStore> BlockchainInterface for ChainState<PersistedSta | |
| self.get_branch_work(header) | ||
| } | ||
|
|
||
| fn acc(&self) -> Stump { | ||
| read_lock!(self).acc.to_owned() | ||
| fn get_acc(&self, block: Option<BlockHash>) -> Result<Stump, Self::Error> { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same thing about |
||
| match block { | ||
| Some(hash) => { | ||
| let height = self | ||
| .get_block_height(&hash)? | ||
| .ok_or(BlockchainError::BlockNotPresent)?; | ||
|
jaoleal marked this conversation as resolved.
|
||
|
|
||
| self.get_roots_for_block(height)? | ||
| .ok_or(BlockchainError::UnknownUtreexoAcc) | ||
| } | ||
| None => Ok(read_lock!(self).acc.to_owned()), | ||
| } | ||
| } | ||
|
|
||
| fn get_fork_point(&self, block: BlockHash) -> Result<BlockHash, Self::Error> { | ||
|
|
@@ -1239,10 +1246,6 @@ impl<PersistedState: ChainStore> UpdatableChainstate for ChainState<PersistedSta | |
| self.reorg(new_tip) | ||
| } | ||
|
|
||
| fn get_acc(&self) -> Stump { | ||
| self.acc() | ||
| } | ||
|
|
||
| fn mark_block_as_valid(&self, block: BlockHash) -> Result<(), BlockchainError> { | ||
| let header = self.get_disk_block_header(&block)?; | ||
| let height = header.try_height()?; | ||
|
|
@@ -1365,7 +1368,7 @@ impl<PersistedState: ChainStore> UpdatableChainstate for ChainState<PersistedSta | |
| .then(|| inputs.clone()); | ||
|
|
||
| self.validate_block_no_acc(block, height, inputs)?; | ||
| let acc = Consensus::update_acc(&self.acc(), block, height, proof, del_hashes)?; | ||
| let acc = Consensus::update_acc(&self.get_acc(None)?, block, height, proof, del_hashes)?; | ||
|
|
||
| self.update_view(height, &block.header, acc)?; | ||
|
|
||
|
|
@@ -1766,7 +1769,9 @@ mod test { | |
| == bhash!("45c74beefa2a110715377e023d4260168b4cafbb0891f3b0869aea30867acc87") | ||
| { | ||
| // This is the block we will reorg to | ||
| fork_acc = chain.acc(); | ||
| fork_acc = chain | ||
| .get_acc(None) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps a |
||
| .expect("tip acc should always be present"); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1789,7 +1794,7 @@ mod test { | |
|
|
||
| assert_eq!(chain.get_best_block().unwrap(), expected); | ||
| assert_eq!( | ||
| chain.acc(), | ||
| chain.get_acc(None).unwrap(), | ||
| fork_acc, | ||
| "The accumulator should not change when accepting headers only", | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ use corepc_types::v30::GetDeploymentInfo; | |
| use floresta_chain::buried_deployments_for; | ||
| use floresta_chain::extensions::HeaderExt; | ||
| use floresta_chain::extensions::WorkExt; | ||
| use floresta_wire::block_proof::TipProof; | ||
| use miniscript::descriptor::checksum; | ||
| use serde_json::Value; | ||
| use serde_json::json; | ||
|
|
@@ -38,6 +39,8 @@ use super::server::RpcChain; | |
| use super::server::RpcImpl; | ||
| use crate::json_rpc::res::GetBlockRes; | ||
| use crate::json_rpc::res::RescanConfidence; | ||
| use crate::json_rpc::res::VerifyUtxoChainTipInclusionProofRes; | ||
| use crate::json_rpc::res::VerifyUtxoChainTipInclusionProofVerbose; | ||
|
|
||
| impl<Blockchain: RpcChain> RpcImpl<Blockchain> { | ||
| async fn get_block_inner(&self, hash: BlockHash) -> Result<Block, JsonRpcError> { | ||
|
|
@@ -243,15 +246,12 @@ impl<Blockchain: RpcChain> RpcImpl<Blockchain> { | |
| .calculate_chain_work(&self.chain)? | ||
| .to_string_hex(); | ||
| let latest_block_time = latest_header.time; | ||
| let leaf_count = self.chain.acc().leaves as u32; | ||
| let root_count = self.chain.acc().roots.len() as u32; | ||
| let root_hashes = self | ||
| .chain | ||
| .acc() | ||
| .roots | ||
| .into_iter() | ||
| .map(|r| r.to_string()) | ||
| .collect(); | ||
|
|
||
| let acc = self.chain.get_acc(None).map_err(|_| JsonRpcError::Chain)?; | ||
| let leaf_count = acc.leaves as u32; | ||
| let root_count = acc.roots.len() as u32; | ||
|
|
||
| let root_hashes = acc.roots.into_iter().map(|r| r.to_string()).collect(); | ||
|
|
||
| let validated_blocks = self.chain.get_validation_index().unwrap(); | ||
|
|
||
|
|
@@ -769,4 +769,49 @@ impl<Blockchain: RpcChain> RpcImpl<Blockchain> { | |
| .map_err(|e| JsonRpcError::Wallet(e.to_string()))?; | ||
| Ok(descriptors) | ||
| } | ||
|
|
||
| pub(super) fn verify_utxo_chain_tip_inclusion_proof( | ||
| &self, | ||
| proof: TipProof, | ||
| verbosity: u8, | ||
| blockhash: Option<BlockHash>, | ||
| ) -> Result<VerifyUtxoChainTipInclusionProofRes, JsonRpcError> { | ||
| // The hash we got querying for the given blockhash | ||
| let internal_hash = blockhash.unwrap_or(self.get_best_block_hash()?); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rust will compute the argument in or u could use
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ill add about |
||
|
|
||
| if proof.proved_at_hash != internal_hash { | ||
| return Err(JsonRpcError::InvalidProof(format!( | ||
| "Possibly stale proof. Got {internal_hash} internally but proof was generated at block {}", | ||
| proof.proved_at_hash | ||
| ))); | ||
| }; | ||
|
|
||
| let stump = self | ||
| .chain | ||
| .get_acc(blockhash) | ||
| .map_err(|_| JsonRpcError::Chain)?; | ||
|
|
||
| let is_valid = stump | ||
| .verify(&proof.proof, &proof.hashes_proven) | ||
| .map_err(|e| JsonRpcError::InvalidProof(format!("Proof verification failed: {e:?}")))?; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Proof error implement
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. StumpError doesn’t implement std::fmt::Display |
||
|
|
||
| match verbosity { | ||
| 0 => Ok(VerifyUtxoChainTipInclusionProofRes::Zero(is_valid)), | ||
| 1 => Ok(VerifyUtxoChainTipInclusionProofRes::One( | ||
| VerifyUtxoChainTipInclusionProofVerbose { | ||
| valid: is_valid, | ||
| proved_at_hash: proof.proved_at_hash.to_string(), | ||
| targets: proof.proof.targets, | ||
| num_proof_hashes: proof.proof.hashes.len(), | ||
| proof_hashes: proof.proof.hashes.iter().map(ToString::to_string).collect(), | ||
| hashes_proven: proof | ||
| .hashes_proven | ||
| .iter() | ||
| .map(ToString::to_string) | ||
| .collect(), | ||
| }, | ||
| )), | ||
| _ => Err(JsonRpcError::InvalidVerbosityLevel), | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -159,6 +159,41 @@ pub struct RpcError { | |
| #[derive(Debug, Deserialize, Serialize)] | ||
| pub struct GetTxOutProof(pub Vec<u8>); | ||
|
|
||
| #[derive(Debug, Serialize, Deserialize)] | ||
| #[serde(untagged)] | ||
| /// Return type for `verifyutxochaintipinclusionproof`, supports both its verbose version and non-verbose. | ||
| /// | ||
| /// The non-verbose version tells whether a proof is valid given the internal utreexo accumulator. | ||
| pub enum VerifyUtxoChainTipInclusionProofRes { | ||
| /// No verbosity, tells whether a proof is valid. | ||
| Zero(bool), | ||
|
|
||
| /// Verbosity one, with more detailed information about the proof | ||
| One(VerifyUtxoChainTipInclusionProofVerbose), | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could have a verbosity level wrapper like: #[derive(Debug, Serialize, Deserialize)]
#[serde(untagged)]
pub enum VerbosityLevel<Z: Debug, Serialize, Deserialize , O: Debug, Serialize, Deserialize> {
Zero(Z),
One(O),
}
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps this suggestion belongs to #1076 and should be applied together with the input type ? The current implementation follows the same pattern as the other methods that have verbosity, and we could work this suggestion for them all in a single pr. |
||
|
|
||
| #[derive(Debug, Serialize, Deserialize)] | ||
| /// Return type for `verifyutxochaintipinclusionproof` | ||
| pub struct VerifyUtxoChainTipInclusionProofVerbose { | ||
| /// Whether this proof is valid | ||
| pub valid: bool, | ||
|
|
||
| /// The block hash that this proof was proved. | ||
| pub proved_at_hash: String, | ||
|
|
||
| /// The targets that this proof is proving. | ||
| pub targets: Vec<u64>, | ||
|
|
||
| /// Hashes count. | ||
| pub num_proof_hashes: usize, | ||
|
|
||
| /// Proof hashes. | ||
| pub proof_hashes: Vec<String>, | ||
|
|
||
| /// Which of the hashes were proven. | ||
| pub hashes_proven: Vec<String>, | ||
| } | ||
|
|
||
| #[derive(Debug)] | ||
| pub enum JsonRpcError { | ||
| /// There was a rescan request but we do not have any addresses in the watch-only wallet. | ||
|
|
@@ -244,6 +279,9 @@ pub enum JsonRpcError { | |
|
|
||
| /// A numeric conversion overflows, e.g., u64 to u32 | ||
| ConversionOverflow(String), | ||
|
|
||
| /// This error is returned when a proof is well-formed but invalid (stale or verification failed) | ||
| InvalidProof(String), | ||
| } | ||
|
|
||
| impl_error_from!(JsonRpcError, MempoolError, MempoolAccept); | ||
|
|
@@ -302,6 +340,7 @@ impl Display for JsonRpcError { | |
| write!(f, "Could not send transaction to mempool due to {e}") | ||
| } | ||
| JsonRpcError::ConversionOverflow(e) => write!(f, "Numeric conversion overflow: {e}"), | ||
| JsonRpcError::InvalidProof(e) => write!(f, "Invalid proof: {e}"), | ||
| } | ||
| } | ||
| } | ||
|
|
||
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.
Would be nice to have tests for the arbitrary block acc.
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.
unitary ? I already included a test for it in the integration tests.