Yeah, In case the local node is a full node, the sync would take nearly the same time.
In case the local node is a pruned node, some blocks would have to be pulled over the network, this sync would be slower and capped by the local internet speed.

TESTING

☑ Successful / unsuccessful claiming works.
☑ Transaction works.
☑ Naive double spend is caught correctly.
☑ Liquidity stack loops working.
☑ Coin loop detection worked ok so far.

2 minor severity issues - both existing in the Natasha version too

Issue 1 - Brain wallet keys not added to the used key feature.

Problem: when creating a brain wallet using the HTTP call, the keys that get
generated aren't added to the used key feature. This means once the keys become
used, node restarted and brain wallet re-generated, it will not be visible that
the used key is already spent.

In function: wallet_generate_brain

Solution: copy the "if enable_used_key_feature" lines from the normal key generator
(key_load_data_internal) into wallet_generate_brain too.

Issue 2 - Used keys balance should be forced to be 0 COMB on the user interface on detected outgoing key spend

Problem: when a key is spent, but the transaction is not added to the node using the link, the node will keep
displaying the old balance - the old key balance (nonzero) will be visible even in the event of 1+  confirmations.

Discussion: This is not a consensus issue, if the user attempts double spending using the nonzero balance,
the double spend will get caught and dropped normally.

In function: wallet_view

Solution: In the wallet printing loop, move the used_key_feature block above printing the wallet key+balance row. Inside the used_key_feature block,
set the balance (bal) to zero if outgoing spend got recognized. Finally, print the outgoing spend row below the walled key+balance row from a temp variables.

While we're fixing this, we may also hide the pay button in case of reorg to save user from accidentally double-spending.
 
Reference wallet.go
https://pastebin.com/raw/cLdG5pB3

Watashi, the program you're working on, am I correct in assuming it's similar to the other program you published a while ago in that it would remove the need for a full node to be running to sync from, and would instead request blocks from peers? If so, then am I also correct in assuming that it will likely take longer to do a full commits build?

Haircomb Core v0.3.4-beta.1 is up here: https://github.com/nixed18/combfullui/releases/tag/0.3.4-beta.1

Build (Optional)

Do the same steps as the normal combfullui (https://bitcointalksearch.org/topic/m.54605575) but with two differences:

1.1. Rather than cloning a github repo, click on "Code" and download the zip file located here: https://github.com/nixed18/combfullui/tree/338d2775d1a5d894259ed1c6b728e251ef432b5a. Move that folder into the location you want to build COMB and extract it, and continue with the build instructions.
 
2. Type in "go get github.com/syndtr/goleveldb/leveldb" to install LevelDB


Set up bitcoin.conf

1. Navigate to the directory where you have stored your BTC blockchain. By default it is stored in C:\Users\YourUserName\Appdata\Roaming\Bitcoin on Windows. You'll know you're there when you see blocks and chainstate folders, along with some .dat files for stuff like the mempool, peers, etc.

2. Look for a file named "bitcoin.conf". If one doesn't exist, make one by right clicking the whitespace and going New>TextFile. Then rename this file to "bitcoin.conf"

3. Open "bitcoin.conf" in Notepad and add the following two entries, replacing XXXXX with what you want your log info to be. This is only used for your BTC node's RPC access.

4. Save and exit


Set up config.txt

1. Navigate to the directory you installed the Haircomb beta.

2. Create a text file called "config.txt".

3. Open "config.txt" in Notepad, and add the following lines, replacing the XXXXX with the same values that you used in your "bitcoin.conf".

4. Save and exit. If Haircomb was open during this edit, you must restart the program for your changes to take effect.


Run

Assuming you're using an unmodded BTC, you can run either BTC or Haircomb first, it doesn't matter. While Haircomb is running, it'll keep checking if there's a BTC server running on your machine, and if so, will attempt to connect with it. When you run BTC, either run bitcoind.exe OR run bitcoin-qt.exe with "-server" as an argument. It is also compatible with Natasha's modded BTC, but just remember to launch Haircomb BEFORE the modded BTC.

Watashi has provided instructions and resources to run the beta using the BTC testnet, those can be found here: https://bitcointalksearch.org/topic/m.56935798


The current version's default port is 2121, this can be changed in the config.txt file with the entry "port=XXXX", replacing XXXX with a valid port number. Selecting the direct reorg handle to test can be done by inserting "reorg_type=miner" into your config.txt

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.