Feat: initial work for tracking p2wsh outputs#6933
Feat: initial work for tracking p2wsh outputs#6933aaronb-stacks wants to merge 5 commits intostacks-network:developfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #6933 +/- ##
============================================
+ Coverage 68.63% 84.66% +16.02%
============================================
Files 412 412
Lines 219013 219194 +181
Branches 338 338
============================================
+ Hits 150330 185571 +35241
+ Misses 68683 33623 -35060
... and 272 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
rob-stacks
left a comment
rob-stacks
left a comment
There was a problem hiding this comment.
LGTM, i would just probably only rename watched_outputs to watched_p2wsh_outputs
| txid: Txid::from_bitcoin_tx_hash(&tx.txid()), | ||
| vout: vout_index | ||
| .try_into() | ||
| .expect("FATAL: parsed bitcoin tx with greater that u32::MAX outputs"), |
There was a problem hiding this comment.
| .expect("FATAL: parsed bitcoin tx with greater that u32::MAX outputs"), | |
| .expect("FATAL: parsed bitcoin tx with greater than u32::MAX outputs"), |
| /// Prune watched outputs whose block height is older than 1.5 reward cycles. | ||
| pub fn prune_watched_outputs(&self, reward_cycle_length: u32) -> Result<(), BurnchainError> { | ||
| // Use integer arithmetic: threshold = (3 * reward_cycle_length) / 2 | ||
| let threshold = (3u64 * u64::from(reward_cycle_length)) / 2; |
There was a problem hiding this comment.
I don't think this is considering the current block height. Should that be added in here?
|
|
||
| Ok(WatchedP2WSHOutput { | ||
| witness_script_hash, | ||
| amount: amount_i64 as u64, |
There was a problem hiding this comment.
I believe this is guaranteed to be safe since all 21M BTC couldn't overflow a u64, but since we generally try to avoid as, could you use a try_from or just add a comment here?
There was a problem hiding this comment.
Yeah, at the very least, this could be a panic.
| pub fn parse_block( | ||
| &self, | ||
| block: &Block, | ||
| block: Block, |
There was a problem hiding this comment.
I don't see why this is necessary. It looks like you could stick with a reference here.
| // Use integer arithmetic: threshold = (3 * reward_cycle_length) / 2 | ||
| let threshold = (3u64 * u64::from(reward_cycle_length)) / 2; | ||
| let sql = "DELETE FROM watched_p2wsh_outputs WHERE block_height < ?1"; | ||
| let deleted = self.sql_tx.execute(sql, [u64_to_sql(threshold)?])?; |
There was a problem hiding this comment.
I think this query is wrong. The block height is absolute, but the computed threshold is not -- it's a height relative to the system start height of Stacks 2.0 (height 666050 on mainnet).
| /// Prune watched outputs whose block height is older than 1.5 reward cycles. | ||
| pub fn prune_watched_outputs(&self, reward_cycle_length: u32) -> Result<(), BurnchainError> { | ||
| // Use integer arithmetic: threshold = (3 * reward_cycle_length) / 2 | ||
| let threshold = (3u64 * u64::from(reward_cycle_length)) / 2; |
There was a problem hiding this comment.
I think this threshold computation is wrong -- it's always a constant value, since burnchain.pox_constants.reward_cycle_length is what gets passed in. I think you also at least want the reward cycle number and the first Bitcoin block height if you're going to use threshold as a block height by which to filter stale p2wsh outputs.
jcnelson
left a comment
jcnelson
left a comment
There was a problem hiding this comment.
I think there's a bug in prune_watched_outputs() that would cause all p2wsh outputs to get deleted every time it's called.
|
@aaronb-stacks please remove @jcnelson as reviewer. |
6 participants
Description
This adds tracking for P2WSH outputs. This is necessary for native bitcoin staking -- in order for lockups to be captured on the burnchain.
This PR allows the stacks-node to track all P2WSH outputs over a fixed window of time, dropping outputs as the window rolls forward.