Add kvstore to catchpoints

[jannotti]

Roundtrip tested in catchpointwriter_test.go, but I'm still unhappy that I'm not testing the entire catchpoint restore process, including replay. A good test that includes that with many kinds of txns would have caught my last bug.

I'm also unsure if the merkle tree is being properly tested with the tests so far.

Sorry for the size of the diff - I finally fixed a problem that was introduced when ledger/internal was created. A lot of handy test code was pushed down into it, even though it was a test of the ledger proper, not ledger/internal (which is mainly the BlockEvaluator). So this moves that support code up into ledger, and uses it (including DoubleLedger) to make it easy to test the catchpoint process after high level txns, like a box creating app call, are issued.

[codecov]

Codecov Report

Merging #4455 (74f2617) into feature/avm-box (d04e00f) will decrease coverage by 0.36%.
The diff coverage is 62.42%.

@@                 Coverage Diff                 @@
##           feature/avm-box    #4455      +/-   ##
===================================================
- Coverage            55.30%   54.93%   -0.37%     
===================================================
  Files                  403      403              
  Lines                51036    51120      +84     
===================================================
- Hits                 28224    28082     -142     
- Misses               20419    20697     +278     
+ Partials              2393     2341      -52     

Impacted Files	Coverage Δ
catchup/catchpointService.go	1.77% <0.00%> (ø)
catchup/ledgerFetcher.go	39.32% <0.00%> (-0.91%)	⬇️
ledger/internal/applications.go	3.70% <0.00%> (-49.10%)	⬇️
ledger/internal/cow.go	64.95% <0.00%> (-13.95%)	⬇️
ledger/internal/eval.go	53.58% <0.00%> (-14.31%)	⬇️
ledger/ledgercore/statedelta.go	7.29% <0.00%> (ø)
util/db/dbutil.go	47.33% <0.00%> (+3.09%)	⬆️
ledger/catchpointtracker.go	61.85% <57.14%> (-1.05%)	⬇️
ledger/accountdb.go	71.96% <63.63%> (-0.07%)	⬇️
ledger/catchpointwriter.go	59.67% <78.43%> (+0.54%)	⬆️
... and 19 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[algorandskiy]

I guess I need another pass and look the entire boxes PR first.
Couple notes:

1. catchpoint dump should have kv values in its textual format
2. on readDatabaseStep and iterators closing business - why it was changed at all?

[algorandskiy]

oh, looks like the feature branch misses catchpoint overflow pr

[tzaffi]

Looks good, but left a few small questions.

[tzaffi]

Suggested change

// TODO: catchpointAccessor.ProcessStagingBalances() like in ledgerFetcher.downloadLedger(cs.ctx, peer, round) like catchup/catchpointService.go which is the real usage of BuildMerkleTrie()

I'm guessing this TODO from #2235 is no longer relevant

@brianolson ?

[jannotti]

I want to merge this into the feature branch, so I'll do that around 4pm unless I hear objections. @algorandskiy , I didn't follow the very last thing you said, but I also see you resolved it, so I'm not going to worry about it unless you mention it again on the feature branch review.

@cce, I'm going to carry over your object to modifiedValue and try to drum up discussion in the feature branch.

[algorandskiy]

I ran a catchpoint test, worked. Going through it when I have time today.

[algorandskiy]

this need to be ordered. The result should be the same across all nodes

[jannotti]

Is this only so that the catchpoint dump utility prints in the same order? I want to check my understanding that the catchpoint files don't actually need to be identical (down to the level of ordering) for any other reason. All that matters for their use that they produce the same merkle trie, which is order independent. (I think.)

I'll certainly make the change, but I want to be sure I know what's going on.

[jannotti]

Oh! This is actually in the catchpoint dump utility! So certainly we're talking about the same reason.

So now, just to be extra safe, can you confirm that the similar SELECT, in catchpointwriter.go does NOT need to be ordered?

	// Create the *Rows iterator JIT
	if cw.kvRows == nil {
		rows, err := tx.QueryContext(ctx, "SELECT key, value FROM kvstore")
		if err != nil {
			return err
		}
		cw.kvRows = rows
	}