Added some new FIAT currencies and enabled auto price update (Default value 300K ms)

[mxaddict]

Added some new FIAT currencies to the QT wallet.

[aguycalled]

this should be thoroughly tested. when it was enabled before it caused the wallet to eventually lose network connections after long periods of use because the sockets descriptors were not correctly released after each request

[mxaddict]

this should be thoroughly tested. when it was enabled before it caused the wallet to eventually lose network connections after long periods of use because the sockets descriptors were not correctly released after each request

Is that a QT related issue? I'll see if I can change it to release the connections

Cause the network calls are using the QT libs

[mxaddict]

I'll compile a version that updates prices every second, and see if it looses connection after a few days.

[aguycalled]

id say deletelater() should release them, but we were seeing that side effect before

[mxaddict]

id say deletelater() should release them, but we were seeing that side effect before

Possibly a bug with the QT 5.7.1 libs? I'll build against both 5.7.1 and 5.9.8 and see if the issue is fixed.

[mxaddict]

id say deletelater() should release them, but we were seeing that side effect before

Looks like I got it to happen, is this the issue? :

2019-05-06 19:16:18 socket send error Bad file descriptor (9)
2019-05-06 19:16:19 INFO: Updating prices
2019-05-06 19:16:19 ERROR: Could not parse price data json 'boost::property_tree::json_parser::json_parser_error'
2019-05-06 19:16:24 INFO: Updating prices

node crashes/closes after the socket send error Bad file descriptor error

Which I think comes from here:
https://github.com/NAVCoin/navcoin-core/blob/59a4ba7486e886c73196b6741b94b229c0d02503/src/net.cpp#L846-L854

[mxaddict]

id say deletelater() should release them, but we were seeing that side effect before

Looks like I got it to happen, is this the issue? :

2019-05-06 19:16:18 socket send error Bad file descriptor (9)
2019-05-06 19:16:19 INFO: Updating prices
2019-05-06 19:16:19 ERROR: Could not parse price data json 'boost::property_tree::json_parser::json_parser_error'
2019-05-06 19:16:24 INFO: Updating prices

node crashes/closes after the socket send error Bad file descriptor error

Which I think comes from here:
https://github.com/NAVCoin/navcoin-core/blob/59a4ba7486e886c73196b6741b94b229c0d02503/src/net.cpp#L846-L854

Seems this was not actually related to the current issue

[mxaddict]

I was able to replicate the issue by setting up a localhost http server to serve the JSON file

And updated the price check interval to 100ms

Node built with QT 5.7.1 fails in about 10 secs

Node built with QT 5.9.8 failes in about 16 minutes

both with same error:

QThreadPipe: Unable to create pipe: Too many open files

[mxaddict]

So I think this behavior should be normal.

My guess is since I was testing with 100ms interval, the QT code that would handle deleteLater() could not keep up with the new requests

So in a more realistic setting were lets say the interval is 5 minutes, this should not be an issue.

@aguycalled @craigmacgregor do you agree?

[mxaddict]

I think this PR is ready for review 😄

[aguycalled]

Has it been confirmed through testing the issue won't happen in a real world scenario with a higher interval, or is it assumed? We saw the issue before with a similar interval.

[mxaddict]

Has it been confirmed through testing the issue won't happen in a real world scenario with a higher interval, or is it assumed? We saw the issue before with a similar interval.

It's assumed as of now (I can't time travel, sorry)

I'll still running the node with the 5 minute interval and will leave it running for a few days to confirm that it does not lose a connection / crash

But I'm almost 💯 % certain that the issue was fixed by moving this line outside the try catch:

reply->deleteLater();

With the old code, it was using QT's Json classes, but I noticed this would hang sometimes (And if it hangs, it would not call the reply->deleteLater()

And over time this would create more and more hung threads (Which I assume still have atleast 1 open files each)

Which after X amount of time (Depending on the OS limits set) the wallet would run out of allowed open files

[mxaddict]

I've done additional testing and found that the wallet indeed does not release the socket files in /proc/<PID>/fd even after the request is run

I'll try to replace the HTTP request code with libcurl instead of using QT5's classes and see if it solves the issue

Sample filesi n /proc/<PID>/fd for a node running at 3 sec update interval:

lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 92 -> 'socket:[759652]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 93 -> 'socket:[765795]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 94 -> 'socket:[765803]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 95 -> 'socket:[763781]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 97 -> 'socket:[767521]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 96 -> 'anon_inode:[eventfd]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 98 -> 'socket:[767572]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 99 -> 'socket:[768311]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 101 -> 'socket:[767611]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 100 -> 'socket:[772218]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 103 -> 'socket:[766455]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 102 -> 'socket:[767633]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 104 -> 'socket:[764853]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 105 -> 'socket:[770109]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 106 -> 'socket:[766510]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 109 -> 'socket:[773138]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 108 -> 'socket:[773124]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:42 107 -> 'socket:[770134]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:43 110 -> 'socket:[768550]'
lrwx------ 1 mxaddict mxaddict 64 May  7 15:43 111 -> 'socket:[768576]'

The HTTP request seem to be the files with socket:[<int>] names

[mxaddict]

@aguycalled @craigmacgregor latest commit in this PR replaces QT's network manager with a plain old curl + std::thread setup.

I can confirm that this implementation was properly releasing the sockets after request was done.

[aguycalled]

utACK

[mxaddict]

@aguycalled I've had a node running these changes for a week, seems to be working and not holding the sockets.

[mxaddict]

@aguycalled The currencies list is getting kinda bloated, maybe I should replace those menu's with a searchable dropdown? Show top 5, then have a search bar?

[aguycalled]

@aguycalled The currencies list is getting kinda bloated, maybe I should replace those menu's with a searchable dropdown? Show top 5, then have a search bar?

ACK

[aguycalled]

Had a node running for 24h+ on Debian

[mxaddict]

I'm adding the GUI change for the coin list ( then you guys can review )

[mxaddict]

• I removed the currency picker in Settings->Currency (Was pretty ugly and redundant)
• Updated currency menu in Options Dialog to be a combobox
• Updated currency menu in bottom right of wallet ui to be a combobox

[mxaddict]

Rebased the branch from master

[mxaddict]

@aguycalled I wanna change my bounty address to XakMNvZREj5XWUTnLJWMqdU62vcoP3ksDY3HAoAEjtKhGLyUSakjLrMjgCuJN for future bounties

[mxaddict]

Rebased from master

[mxaddict]

Demo:

[proletesseract]

building now.

[mxaddict]

@aguycalled you missed a bounty 🤣

[aguycalled]

Paid a07b6e5541ee6c9a249fb59020d573d56bc0a8de26ee853dfde823df68673b79