Options to use HTTPS and to add custom pool-specific headers to DL submission

quetzalcoatl commented 5 years ago

Actually, this could be 2 PRs but during coding session I stacked one change upon the other, it all touches networking areas, so it's probably no use in splitting them into separate updates.

There are two major changes.

Custom headers

First of all, some pools (especially BHD ones) require to send some form of account-id along with the submission. Usually, this means that a 'proxy' must also be used to inject that information, which only makes the learning curve steeper and has no benefits for small miners.

I've extended the config.json with new options, now it is possible to configure the miner to send headers like X-Account or X-MinerName or X-ShoeSize just by editing the config.json. and updated the sample config.json

As a "bonus" I also added a similar section that allows to add extra query parameters in a similar way. I don't know about any pool that would need that, but it was just a "copy-paste" problem and makes the miner a little bit more future-proof.

New config sections are separate for both coins and are pretty simple, like:

    "BHD" : {    
                ...
        "ExtraHeader": {
            "X-Account": "0000-1111-98qr7348errfi8efxny-OK",
            "X-MinerName": "some cool name",
            "X-ShoeSize": "23.5"
        },
                ...

HTTPS support

Now that's a larger update. So far GMI, send_i, confirm_i were using sockets directly and worked over plain HTTP. This causes some serious issues, for example pools hidden behind CloudFlare, when receiving HTTP request, may return HTTP/302 to try to enforce HTTPS. In this case, the miner simply does not handle the redirect and treats it as a failed request, but even if it followed the redirect, it would fail due to lack of support for HTTPS.

With this PR, I've implemented HTTPS connectivity via CURL. I've also deliberately left all of the old code. New CURL code is used only for HTTPS, and the old code is used otherwise. HTTPS is off by default and needs to be explicitely turned on by a single flag:

    "BHD" : {    
                ...
        "UseHTTPS": true,
                ...

and of course the "Port" may have to be updated as well.

Right now, HTTPS is implemented only for the basic mining. "Proxy" mode has not been updated to use HTTPS and still uses existing HTTP code.

Building, testing

I used vcpkg to provide dependencies. I was building it with:

pdcurses 3.6 x64-windows
curl 7.65.0-1 x64-windows

I also did some preliminary testing. Mining works, GMI/send/confirm work over http and https (tested on foxy pool). AccountKey is sent properly (also tested on foxy pool). Miner worked for 8hrs and has not crashed, and has sent all DLs. Survived intermittent 502/badgateway from cloudflare and local network loses. Sounds like working fine.

Performance

I have no idea. I didn't notice anything drastic. but my test plots were small. It shouldn't matter much, as only networking code was changed, but HTTPS will have some impact (handshakes, etc) and also changing from SOCKETS to CURL may have some impact, there are some extra networking buffer allocations as well. I personally wouldn't worry about that, but YMMV.

quetzalcoatl commented 5 years ago

I've just added a pack of patches for some handle leaks. I'm pretty sure those small issues were present in the original code as well.. for example send_i created a socket and added it to tmpSessions.push_back only if everything succeeded, and never released it upon failures, and so on. It was later carried on when I copied code and adapted it to curl. I tested it for ~30hrs and socket count stayed at level of 2..4 vs. ~70 before, so I'm, pretty sure that's I found all of them. Um, except proxy code. I still have not touched it.

quetzalcoatl commented 5 years ago

I'm closing that PR in favor of #5. The new one is not based on my 'master' branch, I totally overlooked that when sending this #4 here.

andzno1 / blagominer