Local Ledger #1011

shiqizng · 2022-05-26T17:41:02Z

Summary

This PR integrates BlockProcessor to daemon and and AddBlock. And AddBlock is refactored to take a validated block.

Test Plan

Regression tests passing.

codecov · 2022-05-26T17:41:10Z

Codecov Report

Merging #1011 (664bdd0) into localledger/integration (f26fca1) will decrease coverage by 0.36%.
The diff coverage is 65.11%.

@@                     Coverage Diff                     @@
##           localledger/integration    #1011      +/-   ##
===========================================================
- Coverage                    59.58%   59.21%   -0.37%     
===========================================================
  Files                           45       46       +1     
  Lines                         8353     8523     +170     
===========================================================
+ Hits                          4977     5047      +70     
- Misses                        2918     3012      +94     
- Partials                       458      464       +6

Impacted Files	Coverage Δ
idb/dummy/dummy.go	`0.00% <0.00%> (ø)`
idb/idb.go	`71.66% <ø> (ø)`
importer/helper.go	`8.92% <0.00%> (-0.51%)`	⬇️
importer/importer.go	`0.00% <0.00%> (ø)`
idb/postgres/postgres.go	`61.37% <70.49%> (-1.98%)`	⬇️
processor/blockprocessor/block_processor.go	`72.66% <71.30%> (-10.68%)`	⬇️
processor/eval/ledger_for_evaluator.go	`78.02% <78.02%> (ø)`
fetcher/fetcher.go	`34.64% <0.00%> (-1.32%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f26fca1...664bdd0. Read the comment docs.

README.md

Eric-Warehime

Made it about halfway through. Left some questions. Overall it's quite a large PR.

Eric-Warehime · 2022-05-31T20:56:23Z

processor/blockprocessor/block_processor.go

+	// validated block
+	var vb ledgercore.ValidatedBlock
+	vb = ledgercore.MakeValidatedBlock(blockCert.Block, delta)
+	if protoChanged {


I see https://github.com/algorand/go-algorand/blob/master/ledger/evalindexer.go which seems to be the reason we need to modify the consensus properties. Is there a reason we need this block just to reconstruct the vb with the updated consensus property?

this block is trying to reproduce the logic implemented in previous version of AddBlock, here. AssetCloseAmount field was added in Algod 2.5.4 release from a year ago, PR #1886, and this override was put in place to support transactions before that release.

processor/eval/ledger_for_evaluator.go

Eric-Warehime · 2022-05-31T21:54:45Z

processor/eval/ledger_for_evaluator.go

+
+// LookupResources is part of go-algorand's indexerLedgerForEval interface.
+func (l LedgerForEvaluator) LookupResources(input map[basics.Address]map[ledger.Creatable]struct{}) (map[basics.Address]map[ledger.Creatable]ledgercore.AccountResource, error) {
+	// Initialize the result `res` with the same structure as `input`.


Why do we need to create a copy instead of mutating the input?

input type doesn't match the return type.

winder

Some initial feedback. I still need to take a closer look at AddBlock

winder · 2022-06-01T21:19:34Z

api/handlers_e2e_test.go

+	l := test.MakeTestLedger("ledger")
+	defer l.Close()
+	proc, err := blockprocessor.MakeProcessor(l, db.AddBlock)
+	require.NoError(t, err, "failed to open ledger")
+	blockCert := rpcs.EncodedBlockCert{Block: block}
+	err = proc.Process(&blockCert)


Could this go in setupIdb instead? Returning a new addBlock callback would be one way to get the tests to behave similar to before, and avoid the duplicate code block. Another way would be to return the processor along with the IndexerDb implementation.

I don't think an addBlock callback would work in this case since addBlock takes a validated block. I'll go with the second option.

cmd/algorand-indexer/daemon.go

cmd/import-validator/core/service.go

winder · 2022-06-01T21:32:43Z

idb/postgres/postgres.go

@@ -263,115 +263,96 @@ func (db *IndexerDb) AddBlock(block *bookkeeping.Block) error {
 		if err != nil {
 			return fmt.Errorf("AddBlock() err: %w", err)
 		}
-		if block.Round() != basics.Round(importstate.NextRoundToAccount) {
+		if block.Round() > basics.Round(importstate.NextRoundToAccount) {


Why aren't these equal anymore? We're ignoring the case where old blocks are submitted more than once?

yeah. I'm making it ignore blocks with older rounds. this change was partly because this test was failing with local ledger, https://github.com/algorand/indexer/blob/develop/idb/postgres/postgres_integration_test.go#L2109. setupIdb calls AddBlock to add genesisBlock but it was already added when calling Makeprocessor().

winder · 2022-06-01T21:38:16Z

idb/postgres/postgres_integration_test.go

+	l := test.MakeTestLedger("ledger")
+	defer l.Close()
+	proc, err := blockprocessor.MakeProcessor(l, db.AddBlock)
+	require.NoError(t, err, "failed to open ledger")
+	blockCert := rpcs.EncodedBlockCert{Block: block}
+	err = proc.Process(&blockCert)


Same as before, can we figure out how to streamline this?

processor/blockprocessor/block_processor.go

processor/eval/ledger_for_evaluator_test.go

README.md

processor/eval/ledger_for_evaluator_test.go

winder · 2022-06-06T23:58:34Z

cmd/algorand-indexer/daemon.go


-				imp := importer.NewImporter(db)
-				handler := blockHandler(imp, 1*time.Second)
+				proc, err := blockprocessor.MakeProcessor(initState, indexerDataDir, imp.ImportBlock)


Could this constructor work with MakeProcessor(&genesis, indexerDataDir, imp.ImportBlock)? It seems like we would want to start fetcher from round 0, so AddBlock could create the initial state when that occurs.

how about pass an algod client to MakeProcessor and skip passing block 0 to handler in MakeProcessor?

updated the method to accept a genesis and a genesis block.

winder

Just left a little more feedback

tzaffi · 2022-06-07T23:37:08Z

processor/eval/ledger_for_evaluator.go

+}
+
+// MakeLedgerForEvaluator creates a LedgerForEvaluator object.
+func MakeLedgerForEvaluator(ld *ledger.Ledger) (LedgerForEvaluator, error) {


Since error is always nil, consider not returning error at all

tzaffi · 2022-06-08T02:12:11Z

processor/blockprocessor/block_processor.go

 	"github.com/algorand/indexer/processor"
+	iledger "github.com/algorand/indexer/processor/eval"


nit: consider renaming iledger → indexerLedger or something a bit more verbose. Through my own misjudgment, I kept on thinking that iledger.xyz are interfaces of some sort.

I'll include these changes in another related pr.

* Local Ledger (#1011) * integrate block processor * Local Ledger Deployment (#1013) * add simple local ledger migration * add deleted opts * fast catchup (#1023) * add fast catchup * Localledger merge (#1036) * return empty lists from fetchApplications and fetchAppLocalStates (#1010) * Update model to converge with algod (#1005) * New Feature: Adds Data Directory Support (#1012) - Updates the go-algorand submodule hash to point to rel/beta - Moves the cpu profiling file, pid file and indexer configuration file to be options of only the daemon sub-command - Changes os.Exit() to be a panic with a special handler. This is so that defer's are handled instead of being ignored. - Detects auto-loading configuration files in the data directory and issues errors if equivalent command line arguments are supplied. - Updates the README with instructions on how to use the auto-loading configuration files and the data directory. * Update mockery version Co-authored-by: erer1243 <[email protected]> Co-authored-by: AlgoStephenAkiki <[email protected]> * recovery scenario (#1024) * handle ledger recovery scenario * refactor create genesis block (#1026) * refactor create genesis block * Adds Local Ledger Readme (#1035) * Adds Local Ledger Readme Resolves #4109 Starts Readme docs * Update docs/LocalLedger.md Co-authored-by: Will Winder <[email protected]> * Update docs/LocalLedger.md Co-authored-by: Will Winder <[email protected]> * Update docs/LocalLedger.md Co-authored-by: Will Winder <[email protected]> * Removed troubleshooting section Co-authored-by: Will Winder <[email protected]> * update ledger file path and migration (#1042) * LocalLedger Refactoring + Catchpoint Service (#1049) Part 1 cleanup genesis file access. put node catchup into a function that can be swapped out with the catchup service. pass the indexer logger into the block processor. move open ledger into a util function, and move the initial state util function into a new ledger util file. add initial catchupservice implementation. move ledger init from daemon.go to constructor. Merge multiple read genesis functions. Part 2 Merge local_ledger migration package into blockprocessor. Rename Migration to Initialize Use logger in catchup service catchup Part 3 Update submodule and use NewWrappedLogger. Make util.CreateInitState private * build: merge develop into localledger/integration (#1062) * Ledger init status (#1058) * Generate an error if the catchpoint is not valid for initialization. (#1075) * Use main logger in handler and fetcher. (#1077) * Switch from fullNode catchup to catchpoint catchup service. (#1076) * Refactor daemon, add more tests (#1039) Refactors daemon cmd into separate, testable pieces. * Merge develop into localledger/integration (#1083) * Misc Local Ledger cleanup (#1086) * Update processor/blockprocessor/initialize.go Co-authored-by: Zeph Grunschlag <[email protected]> * commit * fix function call args * RFC-0001: Rfc 0001 impl (#1069) Adds an Exporter interface and a noop exporter implementation with factory methods for construction * Fix test errors * Add/fix tests * Add postgresql_exporter tests * Update config loading * Change BlockExportData to pointers * Move and rename ExportData * Add Empty func to BlockData * Add comment Co-authored-by: shiqizng <[email protected]> Co-authored-by: [email protected] <[email protected]> Co-authored-by: erer1243 <[email protected]> Co-authored-by: AlgoStephenAkiki <[email protected]> Co-authored-by: Will Winder <[email protected]> Co-authored-by: Zeph Grunschlag <[email protected]>

shiqizng added 25 commits May 12, 2022 09:59

integrate block processor

a96fa70

update ledger path

ed32cb1

add ledger close

8568fa2

refactor AddBlock

fda48cf

api handler test fails

075f265

Merge branch 'develop' into shiqi/AddBlock

ac04503

fixed handler e2e

d7b8012

updating postgres integration tests

4dbda7d

more test updates

c0726b9

refactor processor to use EvalForIndexer

aee8e16

more test refactoring

a12262c

make integration failing

4c15d63

adding ledger for eval tests

7a8c05d

more tests

8de6a18

make integration working

088434b

fix linting errors

dfd2e83

remove debugging lines

a40789f

error fix

1abe9f9

fix test failure

d0fe6a4

refactoring

39dda1b

more refactoring

1e7c0a6

fix failed test

2ca22f3

fix

c4ab181

fix failing test

4d38249

ready for review

6ea76de

shiqizng requested review from AlgoStephenAkiki, Eric-Warehime, winder and algoganesh May 26, 2022 17:41

shiqizng self-assigned this May 26, 2022

shiqizng added the Team Lamprey label May 26, 2022

tzaffi reviewed May 29, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

Eric-Warehime reviewed May 31, 2022

View reviewed changes

winder reviewed Jun 1, 2022

View reviewed changes

chaihoang removed the Team Lamprey label Jun 3, 2022

shiqizng added 9 commits June 3, 2022 19:10

test changes

baaeeb2

fix vet issues

ce32607

fix failing test

c6411ce

fix failing test

27b0a07

update e2e test

d06852d

fix env var

f36732e

add an alias

531588f

use dynamic values in test

93b3988

remove env var alias

2a5cb8b

shiqizng requested a review from winder June 6, 2022 14:30

winder reviewed Jun 6, 2022

View reviewed changes

processor/eval/ledger_for_evaluator_test.go Outdated Show resolved Hide resolved

winder reviewed Jun 6, 2022

View reviewed changes

shiqizng added 4 commits June 7, 2022 15:05

pr reviews

66c3c70

update ledger init

aef1b3b

format readme

b1bc03d

format readme

664bdd0

shiqizng requested a review from winder June 7, 2022 22:45

tzaffi reviewed Jun 7, 2022

View reviewed changes

tzaffi reviewed Jun 8, 2022

View reviewed changes

winder approved these changes Jun 8, 2022

View reviewed changes

shiqizng merged commit 12f8c2f into localledger/integration Jun 8, 2022

shiqizng deleted the shiqi/localledger branch June 16, 2022 20:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Local Ledger #1011

Local Ledger #1011

shiqizng commented May 26, 2022

codecov bot commented May 26, 2022 •

edited

Loading

Eric-Warehime left a comment

Eric-Warehime May 31, 2022

shiqizng Jun 1, 2022

Eric-Warehime May 31, 2022

shiqizng Jun 1, 2022

winder left a comment

winder Jun 1, 2022

shiqizng Jun 3, 2022

winder Jun 1, 2022

shiqizng Jun 2, 2022

winder Jun 1, 2022

winder Jun 6, 2022

shiqizng Jun 7, 2022

shiqizng Jun 7, 2022

winder left a comment

tzaffi Jun 7, 2022

tzaffi Jun 8, 2022

shiqizng Jun 8, 2022

		"github.com/algorand/indexer/processor"
		iledger "github.com/algorand/indexer/processor/eval"

Local Ledger #1011

Local Ledger #1011

Conversation

shiqizng commented May 26, 2022

Summary

Test Plan

codecov bot commented May 26, 2022 • edited Loading

Codecov Report

Eric-Warehime left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winder left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winder left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 26, 2022 •

edited

Loading