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

Log timestamp #91

Closed
wants to merge 1 commit into from
Closed

Log timestamp #91

wants to merge 1 commit into from

Conversation

jgarzik
Copy link
Contributor

@jgarzik jgarzik commented Mar 3, 2011

Add timestamp to each line of debug.log.

@jhyslop
Copy link

jhyslop commented Mar 7, 2011

I like the idea of timestamping log entries. Personally, though, I'd rather see a human-readable timestamp.

@TheBlueMatt
Copy link
Contributor

I agree with jhyslop: TheBlueMatt@fe460d4

@jgarzik
Copy link
Contributor Author

jgarzik commented Mar 9, 2011

That's fine with me. I'll close this pull request, and hope that someone submits the human-readable timestamp patch instead.

Also: there was a privacy concern related to universal timestamps, so maybe add an option -logtimestamp, thereby defaulting the timestamps /off/ by default.

@jhyslop
Copy link

jhyslop commented Mar 10, 2011

Actually, I plan to rework the logging slightly, so I'll add the optional timestamp when I do that. And since I'm adding a flag, there's no reason it can't be '-logtimestamp=<off | numeric | human-readable>'.

What was the privacy concern, do you remember?

My plan is to modify OutputDebugStringF to accept two parameters, one indicating the verbosity level (off, critical error, error, warning, info, debug, verbose) and the other a bitmask enum indicating the area the log entry relates to, such as Mining, Transactions, Blocks, etc. (I got tired of wading through quite literally thousands of IRC log messages, and generating a log file around 1.2M per day). Oh, and then change all 'printf' statements to OutputDebugStringF( x, y where x and y make sense given the context.

I already have that code in my local source, but it's based on the previous release and it's currently hard-coded to Debug, Mining.

@jhyslop
Copy link

jhyslop commented Mar 13, 2011

Jeff, I've been working on the code as mentioned. Can you elaborate on the privacy concerns? I don't really see how the timestamps in the log could be a privacy concern.

@jgarzik
Copy link
Contributor Author

jgarzik commented Mar 13, 2011

Quote from IRC,
Concern was from satoshi... I shouldn't try to read his mind, but I THINK what he was thinking is somebody subpoena's N bitcoin services for their debug.logs to try to figure out where transactions were originating

@jhyslop
Copy link

jhyslop commented Mar 13, 2011

I wondered it might be something along those lines. As a side note, it would be an interesting challenge to figure out how many nodes you'd have to subpoena in order to determine with any degree of certainty the origin of a coin.

My revisions to OutputDebugStringF will, by default, NOT log anything to do with individual transactions. Basically, only infrequent status messages such as the hash rate, warnings, errors and critical errors will be logged by default.

Actually, before I go too far down the road with it, I think I'll open a discussion on the Bitcoin forums to get feedback on my ideas. Maybe someone there might have some additional thoughts on privacy issues.

@jgarzik
Copy link
Contributor Author

jgarzik commented Mar 13, 2011

In general, incremental approaches tend to work better than grand rewrites, so I would rather test the waters with BlueMatt's patch. Then, we can look into something more invasive if the community is happy with the general direction.

@jhyslop
Copy link

jhyslop commented Mar 14, 2011

OK. My changes are orthogonal to BlueMatt's, so it will be simple to merge.

glv2 referenced this pull request in glv2/peercoin Apr 15, 2014
…TestNet build have been consolidated here.
glv2 referenced this pull request in glv2/peercoin Jul 22, 2014
Improve the transaction size (and fee) prediction of coin control.
dexX7 added a commit to dexX7/bitcoin that referenced this pull request Jul 6, 2015
30fdde4 Update package version and resource files (dexX7)
052297b Lay out structure for Omni Core documentation (dexX7)
c628147 Rebrand to Omni Core (file level) (dexX7)
89f19c6 Rebrand to Omni Core (dexX7)
destenson pushed a commit to destenson/bitcoin--bitcoin that referenced this pull request Jun 26, 2016
Updated some old Bitcoin constants and other stuff
classesjack pushed a commit to classesjack/bitcoin that referenced this pull request Jan 2, 2018
…header

Added tests for the state and utxo roots, fixed a typo in CBlockHeader
effectsToCause pushed a commit to vericoin/vericoin that referenced this pull request Jun 22, 2018
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Feb 23, 2019
laanwj added a commit that referenced this pull request Apr 14, 2021
b8e5d0d qt: Handle exceptions in SendCoinsDialog::sendButtonClicked slot (Hennadii Stepanov)
1ac2bc7 qt: Handle exceptions in TransactionView::bumpFee slot (Hennadii Stepanov)
bc00e13 qt: Handle exceptions in WalletModel::pollBalanceChanged slot (Hennadii Stepanov)
eb6156b qt: Handle exceptions in BitcoinGUI::addWallet slot (Hennadii Stepanov)
f7e260a qt: Add GUIUtil::ExceptionSafeConnect function (Hennadii Stepanov)
64a8755 qt: Add BitcoinApplication::handleNonFatalException function (Hennadii Stepanov)
af7e365 qt: Make PACKAGE_BUGREPORT link clickable (Hennadii Stepanov)

Pull request description:

  This PR is an alternative to #18897, and is based on Russ' [idea](#18897 (review)):
  > IMO it would be nice to have a followup PR that eliminated the one-line forwarding methods ...

  Related issues
  - #91
  - #18643

  Qt docs: https://doc.qt.io/qt-5.12/exceptionsafety.html#exceptions-in-client-code

  With this PR the GUI handles the wallet-related exception, and:
  - display it to a user:

  ![Screenshot from 2021-04-01 02-55-59](https://user-images.githubusercontent.com/32963518/113226183-33ff8480-9298-11eb-8fe6-2168834ab09a.png)

  - prints a message to `stderr`:
  ```

  ************************
  EXCEPTION: 18NonFatalCheckError
  wallet/wallet.cpp:2677 (IsCurrentForAntiFeeSniping)
  Internal bug detected: '!chain.findBlock(block_hash, FoundBlock().time(block_time))'
  You may report this issue here: https://github.com/bitcoin/bitcoin/issues

  bitcoin in QPushButton->SendCoinsDialog

  ```

  - writes a message to the `debug.log`
  - and, if the exception is a non-fatal error, leaves the main window running.

ACKs for top commit:
  laanwj:
    Code review ACK b8e5d0d
  ryanofsky:
    Code review ACK b8e5d0d. This is great! I think more improvements are possible but implementation is very clean and I love how targeted each commit is. Changes since last review: adding more explanatory text, making links clickable, reorganizing.

Tree-SHA512: a9f2a2ee8e64b993b0dbc454edcbc39c68c8852abb5dc1feb58f601c0e0e8014dca81c72733aa3fb07b619c6f49b823ed20c7d79cc92088a3abe040ed2149727
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants