Skip to content

Conversation

Empact
Copy link
Contributor

@Empact Empact commented Jan 9, 2019

This is a first step toward a library of common factory functions for test and
bench purposes.

This reduces duplication and supports writing tests by giving easier construction
of the underlying types.

Previously open as #14892.

@fanquake fanquake added the Tests label Jan 9, 2019
@ryanofsky
Copy link
Contributor

You may want to make this a "test_util" library instead of a "factory" library. That would clarify what this code is used for, make this a test-only change that should be easier to merge, and also create a home for other test code that could be reused.


CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CMutableTransaction& txCredit)
{
return BuildSpendingTransaction(scriptSig, txCredit.vout[0].nValue, txCredit.GetHash());
Copy link
Contributor

@practicalswift practicalswift Jan 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aware of the implicit precision losing conversion from long (CAmount) to int here? Should be made explicit or perhaps add an assertion for the allowed value range?

@Empact Empact force-pushed the factories branch 2 times, most recently from 05ae0da to 73fbea2 Compare January 9, 2019 21:51
@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 9, 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:

  • #15045 ([test] Apply maximal flags to tx_valid tests and minimal flags to tx_invalid tests by jl2012)
  • #13926 ([Tools] bitcoin-wallet - a tool for creating and managing wallets offline by jnewbery)

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.

…folder

This is a first step toward a library of common factory functions for test and
bench purposes.
@Empact Empact closed this Jan 17, 2019
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants