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

Rename "block cost" to "block weight" #8363

Merged
merged 1 commit into from Jul 19, 2016

Conversation

Projects
None yet
7 participants
@sdaftuar
Member

sdaftuar commented Jul 18, 2016

Alternate to #8354/#8359. Intended for 0.13.0.

As @laanwj observed, this is somewhat of a tedious change to make, but I do think that it'd be better to make this change now than live with the confusing "cost" terminology indefinitely into the future.

If we go with this, we should re-open bitcoin/bips#420 and make the corresponding change to the BIP.

I didn't change qt/bitcoinstrings.cpp, which -- if I am understanding correctly -- was just changed so that we wouldn't translate the help language for -blockmaxcost. Should I push a commit to this PR that changes the text there as well to refer to "weight", to make this change self-contained?

There's one more language reconciliation we should still do after this, which is that BIP 141 doesn't define "sigop cost", yet we refer to "sigop cost" as a term both in RPC calls (eg getblocktemplate) and throughout the code (comments, variables, function names, etc). I think it'd be best to pick a term, use that in the BIP, and then make a pass through the code to be compatible -- presumably the simplest thing would be to just define "sigop cost" in BIP 141.

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke
Member

MarcoFalke commented Jul 18, 2016

utACK 2c06bae

@luke-jr

This comment has been minimized.

Show comment
Hide comment
@luke-jr

luke-jr Jul 18, 2016

Member

Concept ACK here. BIP 145 also needs updating.

Re sigops, it's IMO simply a bug that Core uses "sigop cost" anywhere, and should be fixed by changing it to either "sigops" or (in comments/help only, not RPC) "BIP141 sigops".

Member

luke-jr commented Jul 18, 2016

Concept ACK here. BIP 145 also needs updating.

Re sigops, it's IMO simply a bug that Core uses "sigop cost" anywhere, and should be fixed by changing it to either "sigops" or (in comments/help only, not RPC) "BIP141 sigops".

@btcdrak

This comment has been minimized.

Show comment
Hide comment
@btcdrak
Member

btcdrak commented Jul 18, 2016

utACK 2c06bae

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Jul 18, 2016

Member

utACK 2c06bae

Member

sipa commented Jul 18, 2016

utACK 2c06bae

@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Jul 18, 2016

Contributor

utACK 2c06bae

Contributor

paveljanik commented Jul 18, 2016

utACK 2c06bae

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jul 19, 2016

Member

Should I push a commit to this PR that changes the text there as well to refer to "weight", to make this change self-contained?

No, better to not update bitcoinstrings.cpp manually, it's supposed to be updated as part of translation updates (make translate).

Member

laanwj commented Jul 19, 2016

Should I push a commit to this PR that changes the text there as well to refer to "weight", to make this change self-contained?

No, better to not update bitcoinstrings.cpp manually, it's supposed to be updated as part of translation updates (make translate).

@laanwj laanwj merged commit 2c06bae into bitcoin:master Jul 19, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Jul 19, 2016

Merge #8363: Rename "block cost" to "block weight"
2c06bae Rename "block cost" to "block weight" (Suhas Daftuar)

laanwj added a commit that referenced this pull request Jul 19, 2016

Rename "block cost" to "block weight"
Github-Pull: #8363
Rebased-From: 2c06bae

@sdaftuar sdaftuar referenced this pull request Jul 21, 2016

Closed

Mining updates for 0.13.0 #8294

5 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment