log: print unexpected version warning in validation log category #19898

n-thumann · 2020-09-06T16:38:15Z

Fixes #19603: As suggested by practicalswift, instead of always printing <n> of the last 100 blocks have unexpected version as a warning appended to UpdateTip, it is now printed in the validation log category and therefore only visible with -debug=validation enabled.

Before:
2020-09-06T15:56:00Z UpdateTip: new best=00000000000000000001b2872e107a98b57913120e5c6c87ce2715a34c40adf8 height=646969 version=0x20400000 log2_work=92.261571 tx=565651941 date='2020-09-06T10:35:36Z' progress=0.999888 cache=32.2MiB(237417txo) warning='72 of last 100 blocks have unexpected version'
After:
2020-09-06T16:31:26Z UpdateTip: new best=0000000000000000000b3bd786dc42745dd7be4a8c695500a04518cb9e2f4dc1 height=646971 version=0x20000000 log2_work=92.261607 tx=565655901 date='2020-09-06T10:57:19Z' progress=0.999883 cache=3.8MiB(27550txo)
2020-09-06T16:31:26Z 71 of last 100 blocks have unexpected version

Ran unit & functional tests, confirmed that the warning is now only printed when validation category is enabled.

src/validation.cpp

hebasto

Concept ACK.

It seems out of scope of this PR, but I think the name nUpgraded is a bit misleading now.

pox · 2020-09-06T19:10:01Z

A blessed change!

Tested on Ubuntu by observing logs with default logging:

2020-09-06T19:01:17Z UpdateTip: new best=0000000000000000000c35da7da08dd7b1521a3ffcbe9efb5e8eedde53d7896b height=647000 version=0x20000000 log2_work=92.262139 tx=565693850 date='2020-09-06T13:53:09Z' progress=0.999892 cache=1.7MiB(12481txo)
2020-09-06T19:02:51Z UpdateTip: new best=0000000000000000000c5a85e128e2a1b6ef8df242c68e3284a5ac29ecaa159a height=647001 version=0x2000e000 log2_work=92.262157 tx=565694885 date='2020-09-06T13:53:38Z' progress=0.999892 cache=3.0MiB(22258txo)
2020-09-06T19:03:27Z UpdateTip: new best=00000000000000000008e73bdaca4a47cd3926d1dcc728238cb7de2bddd014c0 height=647002 version=0x20c00000 log2_work=92.262175 tx=565696981 date='2020-09-06T14:05:46Z' progress=0.999896 cache=4.3MiB(32499txo)
2020-09-06T19:03:57Z UpdateTip: new best=0000000000000000000d2b11cc472ca88e0e84fc9fdf210eff1378ab6f6191fd height=647003 version=0x37ffe000 log2_work=92.262194 tx=565699661 date='2020-09-06T14:22:45Z' progress=0.999901 cache=5.9MiB(43169txo)
2020-09-06T19:04:21Z UpdateTip: new best=0000000000000000000b39032cc59ffe754832bfb7b1bef4dee75a5bef0be502 height=647004 version=0x20000000 log2_work=92.262212 tx=565702102 date='2020-09-06T14:34:51Z' progress=0.999906 cache=7.2MiB(53849txo)

And with -debug=validation:

2020-09-06T19:08:55Z BlockChecked: block hash=0000000000000000000952b5504198cd2cb4d94041f09e3c81ae42ed3bb691c0 state=Valid
2020-09-06T19:08:55Z UpdateTip: new best=0000000000000000000952b5504198cd2cb4d94041f09e3c81ae42ed3bb691c0 height=647025 version=0x20400000 log2_work=92.262597 tx=565750210 date='2020-09-06T18:26:24Z' progress=0.999985 cache=26.4MiB(193591txo)
2020-09-06T19:08:55Z 63 of last 100 blocks have unexpected version
2020-09-06T19:08:55Z Enqueuing BlockConnected: block hash=0000000000000000000952b5504198cd2cb4d94041f09e3c81ae42ed3bb691c0 block height=647025
2020-09-06T19:08:55Z Enqueuing UpdatedBlockTip: new block hash=0000000000000000000952b5504198cd2cb4d94041f09e3c81ae42ed3bb691c0 fork block hash=0000000000000000000d235796b5fbc3df24499d78fcfa4e2ac77173d413de15 (in IBD=false)

practicalswift · 2020-09-06T20:52:41Z

Concept ACK

Thanks a lot for addressing this: this is causing a lot of unnecessary confusion for our users.

Our users are not helped by permanent warnings that are not actionable.

practicalswift · 2020-09-07T19:35:40Z

Tested ACK 7edc068

hebasto

ACK 7edc068, tested on Linux Mint 20 (x86_64) with and without -debug=validation command line option.

I think it is ok to do not translate log messages. bitcoinstrings.cpp will be updated automaGically.

laanwj · 2020-09-08T10:45:39Z

Our users are not helped by permanent warnings that are not actionable.

I've tried to make this point about this log message, but from what I remember I got a lot of pushback last time.
But I still agree.
ACK 7edc068

I think it is ok to do not translate log messages. bitcoinstrings.cpp will be updated automaGically.

Yes, this was a pointless translation, good to see it go. It looks like the other one is translated because it can also end up in the GUI warnings, but this one couldn't anyway.

maflcko

ACK 7edc068

If a softfork is deployed without a BIP9 versionbits compatible warning, it is impossible to write code in advance to detect that scenario with full accuracy. A softfork deployment without a versionbits compatible warning (and especially one that would require most users to upgrade on a short notice) is reckless at best. Putting code into Bitcoin Core to encourage short notice "forced" upgrades seems counter-productive.

Also, the warning is disabled for users that sync up their nodes intermittently (e.g. once a day or once a week).

Finally, for the remaining well prepared users that:

Run a node 24/7
Have -debug or -debug=validation enabled
Read the debug.log
I'd be surprised if this warning had any more impact on their decision to upgrade to a newer version or look out for new versions than the already existing release announcements and our well-known (?) EOL policy.

So I'd say to simply remove this warning.

src/validation.cpp

Instead of printing "<n> of the last 100 blocks have unexpected version" as a warning appended to UpdateTip, it is now printed in the validation log category.

practicalswift · 2020-09-09T19:20:29Z

ACK 62dba96 -- only change since last ACK is s/nUpgraded/num_unexpected_version/

maflcko · 2020-09-09T19:40:52Z

re-ACK 62dba96

though I'd still prefer to remove the warning #19898 (review)

hebasto

re-ACK 62dba96, #19898 (review) is resolved now.

theStack

ACK 62dba96

DrahtBot · 2020-09-19T13:49:51Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

Restore warning for individual unknown version bits, as well as unknown version schemas #15861 (Restore warning for individual unknown version bits, as well as unknown version schemas by luke-jr)

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.

practicalswift · 2020-09-26T15:51:10Z

Is this one ready for merge? :)

I'm counting 4 ACKs (practicalswift, MarcoFalke, hebasto, theStack) and one stale ACK (laanwj).

…ion log category 62dba96 log: print unexpected version warning in validation log category (nthumann) Pull request description: Fixes bitcoin#19603: As suggested by practicalswift, instead of always printing `<n> of the last 100 blocks have unexpected version` as a warning appended to UpdateTip, it is now printed in the validation log category and therefore only visible with `-debug=validation` enabled. Before: `2020-09-06T15:56:00Z UpdateTip: new best=00000000000000000001b2872e107a98b57913120e5c6c87ce2715a34c40adf8 height=646969 version=0x20400000 log2_work=92.261571 tx=565651941 date='2020-09-06T10:35:36Z' progress=0.999888 cache=32.2MiB(237417txo) warning='72 of last 100 blocks have unexpected version'` After: `2020-09-06T16:31:26Z UpdateTip: new best=0000000000000000000b3bd786dc42745dd7be4a8c695500a04518cb9e2f4dc1 height=646971 version=0x20000000 log2_work=92.261607 tx=565655901 date='2020-09-06T10:57:19Z' progress=0.999883 cache=3.8MiB(27550txo)` `2020-09-06T16:31:26Z 71 of last 100 blocks have unexpected version` Ran unit & functional tests, confirmed that the warning is now only printed when validation category is enabled. ACKs for top commit: theStack: ACK 62dba96 MarcoFalke: re-ACK 62dba96 practicalswift: ACK 62dba96 -- only change since last ACK is `s/nUpgraded/num_unexpected_version/` hebasto: re-ACK 62dba96, bitcoin#19898 (review) is resolved now. Tree-SHA512: 2100ca7d6d3fd67c92e81d75162d2506d6f1ecf1761d5180d76663fac06771b35e5c4235ebe1a00731b5f7db82db3cd19328627929c8f22912df592686ba51d3

jonatack

Posthumous ACK 62dba96

bolatovumar · 2020-10-11T19:19:51Z

Thanks for this change! That message is really annoying.

iemwill · 2021-02-24T13:44:27Z

Line 2476 is a good one.

n-thumann mentioned this pull request Sep 6, 2020

Show the "<n> of the last 100 blocks have unexpected version" warning only when running -debug=validation? #19603

Closed

n-thumann force-pushed the log-fix-unexpected-version branch from cbfa32e to 0c02dcb Compare September 6, 2020 16:45

hebasto reviewed Sep 6, 2020

View reviewed changes

src/validation.cpp Outdated Show resolved Hide resolved

hebasto reviewed Sep 6, 2020

View reviewed changes

n-thumann force-pushed the log-fix-unexpected-version branch from 0c02dcb to 7edc068 Compare September 6, 2020 17:37

laanwj added Utils/log/libs Validation labels Sep 6, 2020

hebasto approved these changes Sep 8, 2020

View reviewed changes

maflcko reviewed Sep 9, 2020

View reviewed changes

src/validation.cpp Outdated Show resolved Hide resolved

log: print unexpected version warning in validation log category

62dba96

Instead of printing "<n> of the last 100 blocks have unexpected version" as a warning appended to UpdateTip, it is now printed in the validation log category.

n-thumann force-pushed the log-fix-unexpected-version branch from 7edc068 to 62dba96 Compare September 9, 2020 18:58

hebasto approved these changes Sep 10, 2020

View reviewed changes

theStack approved these changes Sep 12, 2020

View reviewed changes

DrahtBot mentioned this pull request Sep 19, 2020

Restore warning for individual unknown version bits, as well as unknown version schemas #15861

Closed

fanquake mentioned this pull request Sep 21, 2020

fix: remove confusing warning (#16343 #19603) #19987

Closed

fanquake merged commit d82b2c6 into bitcoin:master Sep 29, 2020

hebasto mentioned this pull request Oct 1, 2020

Remove confusing and useless "unexpected version" warning #20054

Merged

jonatack reviewed Oct 6, 2020

View reviewed changes

bitcoin locked as resolved and limited conversation to collaborators Feb 24, 2021

log: print unexpected version warning in validation log category #19898

log: print unexpected version warning in validation log category #19898

Uh oh!

Conversation

n-thumann commented Sep 6, 2020 • edited by fanquake Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

hebasto left a comment

Choose a reason for hiding this comment

Uh oh!

pox commented Sep 6, 2020

Uh oh!

practicalswift commented Sep 6, 2020

Uh oh!

practicalswift commented Sep 7, 2020

Uh oh!

hebasto left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laanwj commented Sep 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

practicalswift commented Sep 9, 2020

Uh oh!

maflcko commented Sep 9, 2020

Uh oh!

hebasto left a comment

Choose a reason for hiding this comment

Uh oh!

theStack left a comment

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Sep 19, 2020

Conflicts

Uh oh!

practicalswift commented Sep 26, 2020

Uh oh!

jonatack left a comment

Choose a reason for hiding this comment

Uh oh!

bolatovumar commented Oct 11, 2020

Uh oh!

iemwill commented Feb 24, 2021

Uh oh!

Uh oh!

n-thumann commented Sep 6, 2020 •

edited by fanquake

Loading

hebasto left a comment •

edited

Loading

laanwj commented Sep 8, 2020 •

edited

Loading

maflcko left a comment •

edited

Loading