the change 2 minutes ago looks good

☑ default credentials are blank
☑ user warned to configure both programs with the same credentials when comms fails
☑ we'll find any bugs during beta testing

feel free to tag the beta!

This is where using the BTC .cookie file makes sense, but the user still needs to provide the path in some way.



I understand this, but I don't see how it'll be possible to completely eliminate the need for config files right now, before the middleware is developed.


The current version of COMB I have up does not require the config to run, only to access the BTC RPC. I'm not sure exactly what you're trying to convey here; if what you're saying is that the current version of COMB MUST be able to access the BTC and pull blocks without any sort of config file, then I'd love to hear how you think it could be done, because I don't see how to accomplish that. Unless you're suggesting a modification to the current program so that it DOESN'T use the RPC, and instead pulls over 8333 then converts to JSON, but that's just the middleware solution.

So again, I'm not 100% sure exactly what you're proposing the steps forwards are; accessing the BTC RPC requires authentication, either the user provides said authentication or they can't pull over the BTC RPC. We can either attempt to make the provision of the authentication easier, or circumvent the RPC via the P2P network. The most non-intrusive way to have the COMB client and the BTC client communicate is using the .cookie file, but COMB needs to know where that file will be. This means that a) the user will have to provide a path to their BTC data dir, via config file or some other means, or b) the user will have to place combfullui WITHIN the BTC data dir. We can set up either of these options, but you seem fairly adamant that COMB should not require a config file in the slightest, which just leaves the user dumping combfullui in the same directory as the .cookie file.

The reason that the BTC call fails with blank creds isn't anything hard coded into COMB; BTC just doesn't return a usable value. Right now the only time that COMB will say "Hey, put in credentials" is if it attempts to contact BTC and gets a bad response back.

EDIT: We could also create an option to submit the BTC datadir path via the COMB UI, and then have COMB save the path to a file in a hidden directory like APPDATA or something. Then the user doesn't have to create a txt file and paste the path in there, but they still have to find the path and paste in in the browser somewhere so I dunno if it's really that much better.

EDIT2: In the same vein as the previous example, we can combine it with your suggestion; on startup COMB generates a random user/pass, then references a path to the bitcoin.conf, and edits bitcoin.conf to contain rpcuser=x and rpcpassword=y. This just seems like a more convoluted version of the cookie solution though, and would also require the user to always start COMB BEFORE BTC, like before.

I will ask you differently, do you wanna take responsibility for angry users
who had their BTC wallets wiped clean by malware running on other computers on their network, because they
have typed "user" & "pass" into their bitcoin.conf, just like your software recommended.

I also considered the solution in which we generate a random long user + pass on startup,
but it's worth shit because it changes every startup and nobody will be arsed to change
it every time (in bitcoin.conf).

What we want is zero configuration. That will be possible by a middle man software. As follows:

1. user downloads bitcoin from bitcoin.org and starts it
2. user downloads a middle man software that don't exist today but will later (once we do it)
3. user downloads haircomb core
4. user starts all 3 programs above
5. haircomb core starts syncing no config needed.

this will be possible because:
1. haircomb core will request blocks from port 8332 (RPC PORT)
2. middle man SW that listens on port 8332 will redirect the requests to port 8333 where the Bitcoin
is already listening on the peer to peer network. (In the correct format). This is true by default
on Bitcoin core, without any configuration.
3. Bitcoin normally transmits the blocks to middleman (in a special format, NOT in JSON)
4. Middleman transits the required blocks to the comb core (in JSON)


What are the possible obstacles to this zero configuration?

1. Bitcoin already running on 8332. The middle man will then fail to run. The middle man program
will recommend to delete both BTC and COMB config files.

2. Old version of comb ( the one we are developing right now) will fail to run without a config file!! (this problem needs to be solved RN)

3. Old version of comb recommending the wrong thing when config file have blank credentials or not found. It must not fail with error and it must keep connecting to the middle man with blank credentials despite of the credentials being blank.

4. Middleman software not being available (this will be solved soon)

5. Blank credentials not being testable - sure, not on BTC, but the fake block simulator will serve you something even with blank credentials.

Right now new_miner_start() runs without credentials, it's the make_bitcoin_call() that stalls out without them. I may be taking your statement too literally here though lol. As far as I am aware, there is no way to connect to BTC over RPC WITHOUT a username and password. There are currently 3 options;

1. Config username and password. What we've been using so far.

2. Cookie file. In the absence of a username and password in its config, BTC will generate a .cookie file that has a username and password to use in it. While we could pull the username and password automatically from said file, we'd still need the user to indicate the path to that file. The only option I see, other than having the user place combfullui in the same folder as .cookie, is to use a config file to communicate the path.

3. RPCAuth. This is just a more complicated version of user/pass that generates a config entry for you, which you then have to place in your bitcoin.conf file.





I pushed the code so its on my github now, sorry I blanked before lol.

blank credentials by default are needed when:

 - when running offline/no reason/intention to sync
 - when syncing using a future tool (to be developed), that tool would run on :8332
   and serve blocks from the Bitcoin P2P network. That won't require the BTC RPC/server
   option, and by extension our config file won't be needed.

in any case new_miner_start() must go, even with blank credentials

the blank credentials cannot be entered into bitcoin's config file. that's exactly
what we need! the user who is capable to edit config files won't be able to do the
wrong thing but will instead be forced to set identical strong credentials in both files.

can't comment on merged main page non synced messages, I don't see the code.

Well that's useful.

I've done all the other stuff, but I don't see how you're getting BTC to run properly with a blank rpc user/pass. If the bitcoin.conf has "rpcuser=" and "rpcpassword=", it's smart enough to know nothing is there and instead spawn and use a cookie file for the relevant info.

Unless I'm missing something, if we want to allow the user to operate COMB without using a config, then we need to set default info (i.e. "user" and "pass") and tell them to insert those into the bitcoin.conf file. I've set it up and modded the error message, but let me know if I'm missing something.

I also just merged the "btc is behind" alert into the "wallet isn't synced" alert, let me know if you think it should be more specific or not.

last things

1. make log file optional and specify the log file name in config.txt, if none or by default don't log.
2. config file should also be optional, we will change the default port to 2121 again, users are used to it.
3. Default rpc password / username should be empty string. It can be used no problem with BTC if you set
that in BTC config (if someone is in rush and doesn't care about security/syncing they won't be making a config file).
4. up the version counter for beta in deployment.go (need to increase it every time when releasing something,
because otherwise there would be versions in the wild that we won't be able to tell what it is later, it's ok to
skip versions)

minor things

message about use 546 sats to claim, change to 330 sats, (it's cheaper).

the message on the main page "COMB height is greater than BTC" is by itself ok but should be reworded,
probably something like "COMB height is different than BTC".

check_btc_is_caught_up() had no boolean result, check it before release, go build should pass

remove fmt.Println(hash), fmt.Println(segments_stack) from stack.go, it just
spams every time I sweep coins. It's not needed.

in general, turn our debug statements that we added so far into log printing or something,

When releasing beta, the less printing on console the better. Except when the coin
crashes. When a new dev joins he/she can put their own debug prints and see them,
not search for their own prints in a pile of spam.

Testnet tools:

To decode bc1 address into witness program:

http://bitcoin.sipa.be/bech32/demo/demo.html

To encode witness program back, but into testnet tb1 address (can be used to claim tesntet comb).

https://bc-2.jp/tools/bech32demo/index.html

Bitcoin command for testnet (prune needs to be high, because with low prune the BTC
will escape from and Comb will get fall behind unable to sync the blocks - "wrong height" ):


Can generate brainwallet for testnet using:

/">http://127.0.0.1:2121/wallet/brain//

We've already synced with testnet. Let's hit faucet, get some testnet bitcoins, and claim some testnet comb.

Holy cow dude, talk about industrious rofl. A lot of this stuff is Golang that I don't know yet, so that's great for me, time to do some learning lol. I guess now I'll start expanding the fake_block code.

One thing to note, your code removed the duration from the WaitForBlock struct, I re-added it in when merging. Looking at the code more it seems like I could've also just removed the duration check from the parse_wait_for_block() function though, I guess it's not exactly used now.

One thing I noticed recently, that may be an issue, every now and then when I was testing stuff in regtest the Haircomb mining loop wouldn't trigger the BTC rpc. It started happening after implementing the new waitfornewblock call; I'm not sure if a problem that's unique to regtest or not, but it's something to look out for. The solution was just to restart the BTC, everything started working again no problem.

I've pushed the code to my github so if anybody else is reading and wants to test it, go right ahead. I've just commented out some of the incompatible commitfile.go and left the rest in there for possible future inclusion of that legacy commit file option you were talking about before.

new  parse_wait_for_block looks good!

I went ahead and implemented the proposed format into the leveldb as well as the use of leveldb.Batch. The leveldb options
are also in use namely to disable compression and enable Sync (this is needed for db to survive power loss - was not tested actually
doing the computer power loss, but should work).

commitfilelvldb.go

https://pastebin.com/raw/c1GDhpAy

mine.go

https://pastebin.com/raw/bM50zwAR

newminer.go

https://pastebin.com/raw/4GrvKpUq

utxotag.go

https://pastebin.com/raw/7rDLYNRY

Overall I've fixed a few bugs:

1. In miner(), when reorging with direction -1 the second topmost block must be compared with previous hash, not the topmost, and the termination condition should be one_or_no_blocks()
2. miner() again, it should Put the block only with direction +1
3. rare race condition in mine_blocks. A local copy of next_download variable is needed to terminate the for loop, because if we use the global one, the goroutines that we spawn may increase that same global next_download, which could then lead to two goroutines to download a certain height and mine the same block twice.


What remains is, use btc_is_caught_up_mutex on the main page, remove mining subrouter s3 from main.go, clear stale code from commitfile.go

You're almost certainly right about this, my lack of computing experience is what led me to assume something that wasn't byte related lol. It'd be idiotic to have a DB that took values in a byte format only to store them in some other ordering.


Oh cool, I didn't know that that was a call that existed. That makes everything much simpler lol.


Maybe the best way to handle this is to just let the user know with a warning, but don't reorg. Similar to how the user gets told that the comb chain is way behind, so balances may look wrong, we can just display a warning like "Commits DB is ahead of the BTC chain. If you are rebuilding the BTC chain please wait for a full resync before operating to make sure your commits are accurate." Or something like that.

EDIT: I slotted in that code, along with the other small fixes mentioned before. Over the next bit I'll do a bit of testing, then move on to refactoring the metadata storage format and processing.

EDIT2: Made a minor change to the code, new parse_wait_for_block() is here: https://pastebin.com/raw/psXFsZbU
If you launch Haircomb before launching the BTC, and then launch the BTC, Haircomb's first (few?) RPC results will be "null", but not be detected as an error. This changes that.

huh I think it just orders in raw bytes, it would be shame to have blocks wrong-ordered IF leveldb sorts by UTF-8.


Yeah those stalls in the GUI are really annoying. I think I fixed it somehow, you know that outer loop in new_miner_start (newminer.go)

This is a standalone test of BTC behavior I needed to check. It can be integrated to comb.

https://goplay.space/#E5EVTWCocOD

Basically I sleep 1/10seconds, then call waitfornewblock RPC. That RPC is almost exactly what we need:

1. it waits until a block added to chain with configurable timeout
2. it returns height+hash in one call! awesome.

One last annoyance is the db completely getting erased when BTC is syncing + at lower height + at the same
chain branch that comb is.

That can be fixed by not reorging in case 1 when btc_is_caught_up=false. Basically we would wait for BTC to reach it's headers height and THEN reorg if needed.

Tell me what you think that is, if there is a worry that the longest valid (heaviest) blockchain would be actually few block shorter than the one comb is on.

I would've thought that the keys would have been ordered in numerical order, not "alphabetical" (so to speak lol), but that makes sense.


The problem with this is that BTC likes to stall on RPC responses if it's busy validating blocks. If all we're doing is alerting the user to the BTC connection status, then I can just modify the current loop to only run if the timestamp has been longer than X seconds so that the other calls can also trigger it.


I have some time now so I'll bang out all of this stuff over the couple of days.


Lol as far as I'm concerned none of it is yet, but I guess for now I'll go with the mining one because that seems like it's more dynamically integrated and could have more shifting variables, so I'm more likely to mess something up that'll affect it and be aware that it's happened

1. metadata and the complexity on initial commits db load is caused by not using
the proper keys for information.

- Block key in database should be just height uint64 in bytes, not prefixed by 99999.
- Commitment key should be the 128bit utxo tag in bytes (it too starts with 64bit height).

Then you can load everything using one NewIterator loop over the whole db because
block and its commitments are sorted by height and are alternating. You don't need any maps/lists or lookups whatsoever.

So when you see a block (a 64bit key) you flush the previous block (if any) and open a new Sha256 writer for checksum.
When you see a commitment (a 128bit key) you keep mining it as well hashing it into checksum writer.
Now, when you see a 1+ higher block you verify the previous block's checksum matches what you squeeze out of Sha256 writer.
Eventually, the whole db will be loaded or you will experience some kind of an error.
Make sure to check the checksum and flush last block when complete database is loaded (if the db contained at least 1 block of course).

In case of an error:

- A new function needs to be written that will clear commit_cache, commit_tag_cache
while commit_cache_mutex is taken.

- delete all commits and the block at the errored height using a new iterator with just that height as prefix.

- Then you should flip to deletion mode, that is continue the original iterator loop,
just keep deleting everything having a 64bit/128bit key at higher than the errored height.

2. the whole ping_btc_loop+miner_command_channel+miner_output_channel thing should be deleted.
It just makes our normal goroutine stuck communicating a pointless thing inside set_connected(false) when I restart BTC.
btc_is_connected should be a mutexed timestamp integer and set_connected(true) should set it to current time each time BTC responds.
Then, if btc_is_connected is a recent timestamp (not older than 2 seconds) we are CONNECTED.

3. slow CommitLvlDbUnWriteVal is still in place

4. get_block_info_for_height was not fixed. It can't contain loop and must get hash from internal source when dir = -1 NOT from the BTC.

5. what about "case 1" (ourhash_at_btc_height == hash)? Use switch u_config.reorg_type to use handle_reorg_direct in that scenario too.

6. When parsing block, we need to copy PBH (string `json:"previousblockhash"`) to the parsed block.
The function miner (func miner) needs to be modified to return error. It will return error when
PBH is not equal to the topmost block hash in case we are mining with direction 1 and there is at least one (previous=topmost) block already.
The above mentioned previous block error needs to be handled in all three places, there are two places in downloader(): handle them using 
stop_run();return. The remaining place is in mine_blocks(), there, just break the loop in case of this error.

7. I don't know whether direct / miner reorg_type should be the default. That depends on you and your confidence about which one is more
production ready.

There was a bug in the metadata loading process; if there were no commits in a block, the block's metadata would be skipped during loading, and the next attempt at pushing a blocks metadata would push a higher block that was expected and cause a crash. I've updated the CommitLvlDbLoad() with a fix, code is here: https://pastebin.com/qt2EGK0y

In the future a full refactor would make sense once we start including headers; first we can load all the metadata into a temp map, checking as we go along that a) the metadata for a block exists, and b) the headers match up and connect properly for the whole chain. Any break in either of those triggers the orphaning to be set at that height. Then we can go back and load the commits, comparing against their appropriate temp-loaded block metadata; unmatching fingerprint means that orphan mark would be triggered. After we've loaded all the non-orphaned commits, and removed any orphans and their block's metadata, we can then load the remaining blocks into their final map.

Actually I guess we could just load the metadata straight into their map and then remove any orphaned ones, that makes more sense.

That's a good idea lol, I hadn't considered just running them both xD

I'm just going to use "reorg_type" for both the struct key and the config.txt entry. Ideally we set one as the default and have the second as a fallback, so that unless it was needed the user wouldn't have to care about the entry. Which do you think makes more sense to be the default? Miner or direct?


These both make sense. The way I had it operating before was that a failed BTC call would trigger the btc_manager to send a signal to the miner that would turn "run" off, and then each loop cycle the get_block_info_for_height() would check that run value, and if it was off then it would shut itself down. Looking back on it though it feels a bit overcomplicated.


Oof yea that's not great. In the future we can jump store the commits but this works for now.


Yea I added this func for handle_reorg for this exact reason, the code I used was




Especially considering you wanted to add like 14 digits to the height number, yea adding 50000000 might not be so effective xD

Although if we're getting rid of the +50000000 method then we should probably just straight up remove the utag_mining_sign(tag) portion of the code; it won't serve any function. We'll need to change the flush detection as well at that point.

With interest I looked at the code.

do we intend to support both bidirectional mining and the new and much more
performant handle_reorg function? I suppose the answer is yes, I mean both
codes do pretty much the same thing and it makes sense to let the user choose.

And, supporting both codes using a config option will mean in case we fuckup
we can just tell users to reconfigure their clients instead of upgrading.

Now here is an example of problem that I have in mind (when using the old bidirectional unmining):

1. get_block_info_for_height() is wrong. That code absolutely must request
the block that needs to be reorged by it's block height and hash read from the db/ram.
Because BTC node might switched to different (better) block chain branch.
At that point a call to getblockhash(500000) will return block 500000 from the better
chain that BTC is on, not from the worse chain that we are on (assuming the reorg is
deeper than 500000).

2. the infinite loop inside get_block_info_for_height() should be removed,
we should just return error on any failure, that will terminate the 6 goroutines and
the downloader will reconsider what to do next after 1 second in the main loop.

But being a new function, I notice the imperfection in handle_reorg(), namely
it corrupts the aes checksum, because it erases commitments in random order,
I think the simplest fix would be to sort each block's commitments by their
utxo tag in decreasing order. So, once temp_commits_map gets populated:


3. I also think I know why the bidirectional unmining is now slow because there is the
function CommitLvlDbUnWriteVal which loops over the entire db I think it can be
eliminated instead:



4. we need to set direction_mine_unmine to UNMINE when reorging (this was previously
done by adding 5000000 to height but thats ugly), in miner_mine_commit_internal:

5. At this point when you set your fake block simulator to quickly reorg blocks,
it will eventually return to the initial situation with checksum of zero

also the database at that point should be cleared. But I still see inside db
the blocks hashes starting at 9999. These need clearing too.

Holy crap, I was planning on making a JSON api where we could just type out fake block info in a text file, but that's way better. That's awesome.

I actually removed the multi-directional mining because I think it makes more sense to reorg based on stored values. In some unknown case where a commit is stored in a block that gets reorged, but the commit isn't real and doesn't exist on the block, the commit won't get unmined. I guess that maybe this won't matter in the end; storing block fingerprints means this should be detected on start up, but just iterating through the commitsdb and removing all commits that have a height above the reorg target seems like the most simple and effective way to go.

The reorg code I'm using atm is here, your instructions were helpful: https://pastebin.com/raw/HFERMEvy. I had to make some changes to the check/find so they're included, as well as the main() so the integration code is there too. The CommitsLvlDbUnWrite is just a delete func based on the given key.

Let me know if you think there's any advantage to having the multidirectional mine option.

EDIT: Implemented the miner, I'll run a speed test tonight after my BTC catches up. Currently using your reorg but like I said, let me know what you think about just pulling straight from the DB.

EDIT2: Modded the DB load to push the block hashes to the map: https://pastebin.com/raw/31ieUe73

EDIT3: Made some minor rough changes to the fake blocker to receive commands. Right now it's just changing the speed it runs at and pausing/starting it; https://pastebin.com/raw/eqZkJTRY

EDIT4: Ran a full build test; the miner took ~10 hours, so it's as fast the the previous versions, and you fixed whatever was wrong with my variation, so yay!

EDIT5: I got the first bit of the fingerprint/block metadata code working. Right now it doesn't check the fingerprint on load, but it does store the block metadata (fingerprint, hash, height) on mine and load it on load. Code's on my github, there were too many scattered changes to drop it in a pastebin. The miner.go have been removed, and the relevant bits merged into newminer.go. Let me know what you think about the straight-to-db reorg vs the multi-dir mine reorg.

EDIT6: Added fingerprint checking, code's on the github. There's a TON of datatype switching that can be optimized/removed later, but it works for now. I also modded the initial fingerprinting so that now the final fingerprint includes the hash of the block too.

okay if you can't batch unwrite, we will need to clean up at startup.

take a look at this newminer.go

https://pastebin.com/raw/2RCREVsy


and here is a test RPC server that serves test blocks, the initial height is 500000

https://goplay.space/#2TGEZUC9ezM

It doesn't look like you can batch unwrite with this leveldb, just batch write. Let me know if I'm missing something. It should be fine; if the first thing removed from the commitsdb is the associated headers, then even if the process crashes mid-reorg the on-start orphan clean-up should take care of the rest. I'll just pull the relevant commit keys when I'm iterating through the commits map, then remove them from the commitsdb afterwards one by one.

I'm also not sure what you mean by "2. make the  utxo tag correspond to the target height, set it's txnum=0, outnum=0, direction=false." What utxo tag? EDIT: Do you mean the commit_currently_loaded?

I'm assuming you mean the tx_leg code in the the commits_rollback section, not the commits_cache section, correct?

This won't unmine in order of commits within the same block, but that's obvious so I'm assuming its fine.

EDIT2: I got the reorg working, but I ran a full pull and something's wrong with the way I implemented the mining code it seems like, so I'll have to figure that out.

what needs to happen when reorg back to a specific height (target height):

1. lock the commit_cache_mutex and commits_mutex,
2. make the  utxo tag correspond to the target height, set it's txnum=0, outnum=0, direction=false.
3. Run a for loop from the max height down towards target height:
4. - loop over the commits map. For each commit (key) whose height is equal to the currently unrolled height:
5. - - delete it from the combbases map, if it was there also call segments_coinbase_unmine() at that commit and unrolled height.
6. - - delete it from the commits map.
7. - - if used keys feature is enabled call used_key_commit_reorg(key, currently_unrolled_height)
8. - - call merkle_unmine(key)
9. - - also call the block of code in mine.go starting with txleg_mutex.RLock() and ending with txleg_mutex.RUnlock(), probably refactored to a separate function.
10. - - set unwritten = true if unwritten at least 1 commit
11. - don't do the thing below for every commit (key) anymore, but just for every height:
12. - if unwritten && enable_used_key_feature {used_key_height_reorg(reorg_height);}
13. don't do the thing below for every height anymore, but just once for the entire long range reorg:
14. commit_rollback = nil // to be sure
15. commit_rollback_tags = nil // to be sure
16. lazyopen = false // to be sure
17. resetgraph() // to reflow the entire balance graph
18. Truncate from LEVELDB everything above target height using a batch.
19. adios (unlock the 2 mutexes from step 1)


the nice thing is that you will be able to reorg back to any height, calling this new function once, not just 1 block back.