[Bug] burn ops are applied multiple times per tenure

[jcnelson]

This code is buggy:

        let (stacking_burn_ops, transfer_burn_ops, delegate_burn_ops, vote_for_agg_key_ops) =
            if new_tenure || tenure_extend {
                StacksChainState::get_stacking_and_transfer_and_delegate_burn_ops(
                    chainstate_tx,
                    &parent_index_hash,
                    sortition_dbconn.sqlite_conn(),
                    &burn_header_hash,
                    burn_header_height.into(),
                )?
            } else {
                (vec![], vec![], vec![], vec![])
            };

We should only be loading up the on-burnchain operations if the burn view has changed. This neither corresponds to tenure-change nor tenure-extend blocks.

[kantai]

get_stacking_..._burn_ops() does attempt idempotency -- however, the logic that works for 2.x no longer works on 3.x:

In stackslib/src/chainstate/stacks/db/blocks.rs:

        let processed_burnchain_txids = StacksChainState::get_burnchain_txids_in_ancestors(
            chainstate_tx.deref().deref(),
            parent_index_hash,
            search_window.into(),
        )?;

        // Find the *new* transactions -- the ones that we *haven't* seen in this Stacks
        // fork yet.  Note that we search for the ones that we have seen by searching back
        // `BURNCHAIN_TX_SEARCH_WINDOW` *Stacks* blocks, whose sortitions may span more
        // than `BURNCHAIN_TX_SEARCH_WINDOW` burnchain blocks.  The inclusion of txids for
        // burnchain transactions in the latter query is not a problem, because these txids
        // are used to *exclude* transactions from the last `BURNCHAIN_TX_SEARCH_WINDOW`
        // burnchain blocks.  These excluded txids, if they were mined outside of this
        // window, are *already* excluded.

There's simpler ways to do this with the 3.x chainstate metadata -- if you just compare the burn block parent to the current burn view, it should work.

[jcnelson]

Yeah, I don't think this is terribly difficult. I'm going to treat the index_block_hash column in burnchain_txids as the tenure-start block ID, and search back N tenures instead of N Stacks blocks. Also, I'm only going to call get_burnchain_txids_in_ancestors() only if there's a tenure-change. This would make the Nakamoto handling equivalent to the Stacks 2.x handling.

[blockstack-devops]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.