Skip to content
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

refactor: Expose UndoReadFromDisk in header #15623

Merged
merged 1 commit into from Mar 20, 2019

Conversation

@MarcoFalke
Copy link
Member

commented Mar 19, 2019

It is not possible to calculate the fee of a non-mempool transaction in RPCs unless txindex is active or the prevtxs are passed in through the RPC.

Fix that issue for confirmed txs by exposing UndoReadFromDisk in the header file.

This pull is a requirement for

  • rpc: faster getblockstats using BlockUndo data #14802
  • Index for BIP 157 block filters #14121
  • my local patches
Copy link
Member

left a comment

utACK faad977

@practicalswift

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

utACK faad977

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #14121 (Index for BIP 157 block filters by jimpo)
  • #14053 (Add address-based index (attempt 4?) by marcinja)

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.

Copy link
Contributor

left a comment

Concept ACK

@@ -1515,6 +1513,8 @@ static bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex *pindex)
return true;
}

namespace {

This comment has been minimized.

Copy link
@jimpo

jimpo Mar 19, 2019

Contributor

Is this anonymous namespace still necessary? The two methods inside it are both static.

@MarcoFalke MarcoFalke force-pushed the MarcoFalke:1903-UndoReadFromDiskHeader branch from faad977 to fa11c03 Mar 19, 2019
@MarcoFalke

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2019

Removed namespace, as requested by @jimpo

@jamesob

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

re-utACK fa11c03

@jimpo

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

utACK fa11c03

1 similar comment
@FelixWeis

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

utACK fa11c03

@practicalswift

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

utACK fa11c03

Copy link
Contributor

left a comment

utACK fa11c03, makes sense

@MarcoFalke MarcoFalke merged commit fa11c03 into bitcoin:master Mar 20, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
MarcoFalke added a commit that referenced this pull request Mar 20, 2019
fa11c03 refactor: Expose UndoReadFromDisk in header (MarcoFalke)

Pull request description:

  It is not possible to calculate the fee of a non-mempool transaction in RPCs unless txindex is active or the prevtxs are passed in through the RPC.

  Fix that issue for confirmed txs by exposing `UndoReadFromDisk` in the header file.

  This pull is a requirement for
  * rpc: faster getblockstats using BlockUndo data #14802
  *  Index for BIP 157 block filters #14121
  * my local patches

Tree-SHA512: 859ea5f2dfb4feac612b50faeb0e2b6c07b83f1d983e519d7647a78058d85c0390fd09ec66b460ae7a4c3b273e81b0013ee9f4bb8dfba0c4782ffaa1fa453ea6
@LitecoinZ LitecoinZ referenced this pull request May 31, 2019
44 of 244 tasks complete
@MarcoFalke MarcoFalke deleted the MarcoFalke:1903-UndoReadFromDiskHeader branch Jun 11, 2019
luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Sep 3, 2019
Github-Pull: bitcoin#15623
Rebased-From: fa11c03 (diff minimised)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.