Refactor indexing #1302

Tangui-Bitfly · 2025-01-29T16:07:53Z

The main goal of this PR is to remove functionalities from db package and put them into there own object "easily" testable. Those changes aim at removing the basic indexing flow.
⚠️ Please do not focus on the code quality, I tried to change as little as possible to reduce risk of breaking something. The goal here is not to refactor everything but to separate things, remove coupling, introduce dependancies injections and a tests.

Overview of the new architecture

pkg
    commons
        db2
            database            // wrappers around database interractions
                databasetest    // test utility that provide an in mem database that database wrappers can use
            data                // handles all the interactions with the store `data` (indexing resulted objects)
            metadaupdates       // handles all the interactions with the store `metadaupdates` (balances, contracts, ...)
    executionlayer              // contains the indexer and the transformers

Those are the main package to focus your review on.
Lot of the changed lines come from generated code around contract bindings. You can find those in internal/contracts, and I would suggest to not spend too much time on those beside maybe having a look at the readme here internal/contracts/README.md.

backend/cmd/store/main.go

backend/pkg/commons/db2/data/keys.go

backend/pkg/executionlayer/transformer.go

backend/pkg/commons/db2/data/data.go

mccreedy-bitfly

Generally looks OK. I would like to note that this CR is enormous, and so couldn't review it for functional changes. Since this review concentrates mostly on testing, I focused my efforts there, specifically on the structure and ensuring we were creating a good model that others could use moving forward.

Your suggestion to use table-based tests is a good one, and raises the quality and our confidence of the code as a whole. I have concerns however about the flexibility of the testing code if we were to for example change the underlying database model, and how much of these tests we would have to re-write. However, as we had 0 tests prior to this CR and because this is clearly already a measurable improvement, I believe that is out of scope and we can tackle that problem later.

Please avoid having "TODOs" in the code, as it is very unlikely we will go back and revisit them. Consider any TODOs as essentially final. Also, next time it would make sense to split such a large CR up in to smaller pieces, so that they are reviewable and can be iterated on more quickly. This saves time for both the author of the changes and the reviewers.

mccreedy-bitfly · 2025-02-06T14:13:44Z

backend/pkg/executionlayer/transformer_test.go

+		},
+		// TODO add transfer bulk when bug fixed
+	}


Whats this?

It's related to a bug I found but did not fix.
https://github.com/gobitfly/beaconchain/blob/refactor-indexing/backend/pkg/executionlayer/transformer.go#L398
To sum up, erc1155 can be transferred in batch, in that case we only save the last batched transfer because we overwrite our indexed log for each batch elem.
There is no easy fix to that because we store those indexed log with key <tx_index>:<log_index> but in this case all the batched transfer have the same log index. So even if we fix our indexing, it will be overwritten in the db.

mccreedy-bitfly · 2025-02-06T14:14:22Z

backend/pkg/commons/db2/data/data_external_test.go

+	/*
+		list of filter
+		data.ByMethod(method)
+		data.ByAsset(asset)
+		data.OnlyReceived()
+		data.OnlySent()
+		data.IgnoreTransactions()
+		data.IgnoreTransfers()
+		data.WithTimeRange(from, to)
+	*/


Why commented out code?

it was more a test to showcase how to interact with the data.Store, I can remove it bring confusion.

mccreedy-bitfly · 2025-02-06T14:20:22Z

backend/pkg/executionlayer/transformer.go

+		// BaseFeeChange:          new(big.Int).Sub(new(big.Int).SetBytes(block.GetBaseFee()), new(big.Int).SetBytes(previous.GetBaseFee())).Bytes(),
+		// BlockUtilizationChange: new(big.Int).Sub(new(big.Int).Div(big.NewInt(int64(block.GetGasUsed())), big.NewInt(int64(block.GetGasLimit()))), new(big.Int).Div(big.NewInt(int64(previous.GetGasUsed())), big.NewInt(int64(previous.GetGasLimit())))).Bytes(),


copy pasted previous code

mccreedy-bitfly · 2025-02-06T14:20:26Z

backend/pkg/executionlayer/transformer.go

+		GasUsed:    block.GetGasUsed(),
+		Time:       block.GetTime(),
+		BaseFee:    block.GetBaseFee(),
+		// Duration:               uint64(block.GetTime().AsTime().Unix() - previous.GetTime().AsTime().Unix()),


copy pasted previous code

Tangui-Bitfly force-pushed the refactor-indexing branch 3 times, most recently from 31274d8 to b06f30c Compare January 30, 2025 14:47

Tangui-Bitfly marked this pull request as ready for review January 30, 2025 14:51