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

Usability: During initial sync its not obvious that txn will not be displayed #1236

Closed
gmaxwell opened this issue May 9, 2012 · 12 comments
Closed

Comments

@gmaxwell
Copy link
Contributor

gmaxwell commented May 9, 2012

Report from IRC:

< Eliel> I think something needs to be done to the user interface to help users avoid situations like this. I wonder how many have just silently just abandoned bitcoin after trying and not seeing the transaction in their client.
http://www.reddit.com/r/Bitcoin/comments/tcy73/help_i_withdrew_funds_from_mtgox_gave_them_my/
< Eliel> it's not clearly visible from the UI that you can't expect transactions to show up before the blockchain download is complete. There's also plenty of people who don't read instructions.
< Eliel> I think a good way to fix this would be to show a note on the top of the transaction list that says "blockchain still downloading, some transactions may not show up"

@gmaxwell
Copy link
Contributor Author

gmaxwell commented May 9, 2012

Challenge here is that probably should only be emphasized during the initial sync— but how to avoid the user thinking the absence of the message means its totally synced and everything should be displayed?

@laanwj
Copy link
Member

laanwj commented May 9, 2012

Right, the warning message should be shown when either the last block is older than 90 minutes (when the spinner shows) or when there is no network connection.

The challenge here is really coming up with a good message. It should make clear that transactions could be missing, but that absence of the message doesn't guarantee that no transactions are missing.

@jojkaart
Copy link

Perhaps rather than a message that either shows up or not, show a timer that estimates how far behind the rest of the network the client might be? Reset the timer on every block or received transaction. Perhaps color the timer red and show the message if the timer goes past a certain limit value like 10 minutes.

@laanwj
Copy link
Member

laanwj commented May 11, 2012

A timer would be possible, but it will be much more imprecise than that. Blocks are generated every 10 minutes on average, but holes of up to 90 minutes still happen regularly.

@Diapolo
Copy link

Diapolo commented May 11, 2012

If the chain is not up-to-date just display a small but noticeable lable with a good info message? No timer, just check if we are fully synced.

@laanwj
Copy link
Member

laanwj commented May 15, 2012

This warning should be shown on the overview page, probably in big red letters.

After all, it's not just the transactions that aren't complete. But the balance can be wrong as well.

@Diapolo
Copy link

Diapolo commented May 15, 2012

Working on a GUI warning for this .. see #1314!

@Diapolo
Copy link

Diapolo commented May 17, 2012

@gmaxwell Are you okay with this message as tooltip: The displayed information reflects an older state. To get more recent information, the local block chain needs to be synced with the network. This process starts automatically after a network connection is established. See #1314 for details.

@gmaxwell
Copy link
Contributor Author

Nitpicking: "state" is a bit of technobabble perhaps. "To get more" starts of sounding like instructions to the user to take action. Maybe something more like "The displayed information may be out of date. Your wallet automatically synchronizes with the Bitcoin network after a connection is established, but this process has not completed yet."

I'm not too concerned with the details. Let me see if I can prod wumpus on irc.

@Diapolo
Copy link

Diapolo commented May 17, 2012

@gmaxwell I will update once the final sentences are finished :).

@laanwj
Copy link
Member

laanwj commented May 17, 2012

/agrees with gmaxwell's message

@Diapolo
Copy link

Diapolo commented May 17, 2012

Updated the pull.

@laanwj laanwj closed this as completed Sep 21, 2012
lateminer pushed a commit to lateminer/bitcoin that referenced this issue Jan 22, 2019
* main: Removing never executed code regarding an old client version

A version value of 10300 is below the MIN_PEER_PROTO_VERSION,
which will generate an error already further up in the code.

* Extended Version Message implementation

This is a proposal and implementation for augmenting the `version`
message in the BCH protocol with an `xversion` message to provide
a key value map of 64bit integers to `std::vector<uint8_t>` values
(called `xmap`) that can be used to signal general configuration
and version options to connected peers.

This implementation comes with a specification document in
doc/xversionmessage.md, a simple unit test and a fuzzer test
case. It also comes with a proposal on how to organize and
name the `xmap` key space between implementations, as well as a tool
to process a key space definition file (xversionkeys.dat) into
a C++ header.

Furthermore, the BU-specific BUVERSION message has been retired (but
is still handled) and replaced with an appropriate entry in this map.

Thanks to everyone who gave me input on this PR:

@gandrewstone, @Greg-Griffith, @dgenr8, @sickpig, @dagurval,
@jasonbcox, @schancel, @deadalnix

* Rework of the initial connection handshake

This is a rather comprehensive rework of the initial connection
set up logic. Instead of having various flags that track the individual
sending and receiving of messages, the set up sequencing for a connection
has been replaced with two state machines and corresponding state
variables (state_incoming, state_outgoing) that track the status of
the incoming and outgoing connection setup parts, and of sending
any kind of version message (version, buversion, xversion) around.

This change comes with a few behavioral changes and thus needs to be
tested comprehensively in a test net as well as live nodes for reasonable
certainty as to bug scarcity.

The incoming and outgoing state variables and their allowed
transitions are as follows:

For INCOMING versioning message from the remote peer, using
the ConnectionStateIncoming enum as defined in net.h:

CONNECTED_WAIT_VERSION: Initial state. The TCP connection is up and
the node is waiting for the version message from the remote peer. Only
a version message is valid here. All other messages are invalid and
will either lead to disconnection and banning or are silently ignored.

SENT_VERACK_READY_FOR_XVERSION: After receiving the version message (and
sending the verack out in the event handler). Only now it is permissible
to receive and extended version message 'xversion' from the remote peer.
If any other message comes in, the state transitions to READY automatically,
which will make receipt of xversion messages invalid.

READY: The version handshake is complete and normal message exchange can
commence.

For OUTGOING versioning messages to the remote peer, using the
ConnectionStateOutgoing enum as defined in net.h:

CONNECTED: Initial state after TCP connection is up.

SENT_VERSION: The initial version message has been sent to the remote peer.

READY: The verack has been received and the buversion as well as xversion
messages have been sent to the remote peer.

Any regular message that is coming in the initial version message, before both
incoming and outgoing connection state transitioned to "READY" will be dropped now.

* toString(..) function for printing/logging of enums

This function will take an enum (cast to uint64_t) and a map
of bit values to strings and produce a string describing the OR-ed
enum value.

To be used in the following xversion commits.

* test_framework: Allow to set pushbuf for higher level send_message(..)s

Allow to set the pushbuf argument also in the send_message(..)s
methods that build on top of the basic one.

* test_framework: Add node_regtest_dir(..) function

To allow for accessing files other than the log file in a node's
working directory as well.

* nodemessages: Add CompactSize de/ser

There's some duplicated code handling with the compact integer format
that can now be reworked to use this implementation. However, this commit
doesn't contain the rework yet - it just adds one more implementation ...

* test_framework: Add xversion and xverack message support

Add xversion / xverack messages to mininode and handle their receipt in a
simply but likely sufficient way. No changes have been made to the sending
side of the mininode in here.

* RPC test xversion.py for xversion handshake

Simple functional test for the new xversion handshake. Also adds this to
the default RPC test list.

* Disable parallel processing for extended version handshake

Only enable parallel message processing when all version information has
been exchanged.

* test_framework/mininode: More info on message handling errors

* sendheaders: Fixes for xversion

Wait for proper xversion setup before testing the sendheaders functionality.

* [minor] test_framework: Remove superfluous 'pass' statement

* test_framework: Fix repr() of bu_version class

* test_framework: Make sending of initial version message optional

For testing this part of the p2p handshake.

* Makefile: Include xversion infrastructure as EXTRA_DIST
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants