Skip to content

Conversation

JeremyRubin
Copy link
Contributor

@JeremyRubin JeremyRubin commented Jul 14, 2021

Previously, the Checkers stored pointers that were ambiguously null or not for the Transaction and the Precomputed Data. This PR makes the interface always require a reference to a transaction and to a PrecomputedData and remove ambiguity.

This in turn eliminates some dereferences that were previously not asserted locally, and generally makes the APIs more clear.

This doesn't reduce the ability to not precompute data as PrecomputedTransactionData has it's own internal initialization process -- if that is desired, an unitialized PrecomputedTransactionData can be used. This PR does not attempt to distinguish places where an unfilled PrecomputedTransactionData could be used -- for the most part, these are non performance sensitive signing code paths.

Note that the internal representation for CScriptCheck does not change because there is a need to swap pointers (and not the underlying data). To get rid of the pointers altogether, a reference_wrapper could be used.

fixes #22438

@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 14, 2021

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #22275 (A few follow-ups for taproot signing by sipa)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot
Copy link
Contributor

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".

@DrahtBot
Copy link
Contributor

There hasn't been much activity lately and the patch still needs rebase. What is the status here?
  • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
  • Is it no longer relevant? ➡️ Please close.
  • Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

@maflcko
Copy link
Member

maflcko commented May 6, 2022

Looks like this was picked up in #22793, so this can be closed?

@maflcko
Copy link
Member

maflcko commented May 6, 2022

Closing for now due to inactivity for more than a year. Happy to reopen if you work on this again.

@maflcko maflcko closed this May 6, 2022
@bitcoin bitcoin locked and limited conversation to collaborators May 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make PrecomputedData hold references instead of pointers?
3 participants