Skip to content

Conversation

luke-jr
Copy link
Member

@luke-jr luke-jr commented Jun 15, 2017

No description provided.

@sipa
Copy link
Member

sipa commented Jun 16, 2017

utACK 4292a75

@luke-jr
Copy link
Member Author

luke-jr commented Oct 15, 2018

Rebased. At this point, it might make sense to just drop support for pre-segwit miners, but this fix is simple, so it makes sense to get in (and backportable) first.

Copy link
Contributor

@jnewbery jnewbery left a comment

Choose a reason for hiding this comment

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

I agree that we should just drop support for non-segwit miners at this point. Doesn't seem worth the trouble of making this change.

@@ -225,6 +225,13 @@ def run_test(self):
# Now try calling getblocktemplate() without segwit support.
template = self.nodes[0].getblocktemplate()

# Check that we got a pre-Segwit template
assert(template['sizelimit'] == 1000000)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: prefer assert_equal() for asserting equality

@@ -225,6 +225,13 @@ def run_test(self):
# Now try calling getblocktemplate() without segwit support.
template = self.nodes[0].getblocktemplate()

# Check that we got a pre-Segwit template
assert(template['sizelimit'] == 1000000)
assert('weightlimit' not in template)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: assert is a statement, not a function. Please remove parens.

@DrahtBot
Copy link
Contributor

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #14811 (Mining: Enforce that segwit option must be set in GBT 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.

@maflcko
Copy link
Member

maflcko commented Dec 21, 2018

This has been "fixed" by

@DrahtBot
Copy link
Contributor

Needs rebase

@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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants