Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: break up transformer and rpc package logic into smaller func and add unit tests #1313

Open
wants to merge 56 commits into
base: staging
Choose a base branch
from
Open

refactor: break up transformer and rpc package logic into smaller func and add unit tests #1313

wants to merge 56 commits into from
+2,811 −419

Conversation

Monika-Bitfly
Copy link

Changes:

  • Refactored transformer.go functions in the /executionlayer package by breaking them into smaller, more readable functions
  • Simplified erigon.go and geth.go functions in the /rpc package by splitting them into smaller functions and introduced utils.go to share common functionality between them
  • Added unit tests utils_test.go and erigon_test.go in the /rpc package
  • Added unit tests transformer_test.go in the /executionlayer package

Tangui-Bitfly and others added 30 commits January 30, 2025 15:45
@Tangui-Bitfly
refactor(execution layer): extract el indexing it own packages
b06f30c
@Tangui-Bitfly
chore: contract simplification
df7c389
@Tangui-Bitfly
feat(database transaction): add transaction to database package
504de9a
@Tangui-Bitfly
fix: rename database.Transaction to database.Batch
6b06d7b
@Tangui-Bitfly
fix: check max uncles len
0bf7bbf
@Monika-Bitfly
refactor: extract logic into separate functions for improved readability
3f74ed5
@Monika-Bitfly
refactor: break down transformer logic into smaller func for improved…
cae25cb 
… redability
@Tangui-Bitfly
refactor(indexer): remove even more dependancies with database
d14bd52
@Monika-Bitfly
refactor: break down erigon and geth logic into smaller func for impr…
2efbee2 
…oved redability
@Tangui-Bitfly
add test for erc20 transform
ae0831f
@Monika-Bitfly
refactor: add utils.go for code reuse
1634979
@Monika-Bitfly
Merge branch 'refactor-indexing' of github.com:gobitfly/beaconchain i…
d767adb 
…nto m/refactor-indexing-v2
@Monika-Bitfly
test: add tests for getTxRecipient, getMethodSignature & updateITxStatus
0d4bb82
@Monika-Bitfly
test: add tests for isValidERC20Log, getLogTopics & getERC20TransferV…
9f1af82 
…alue
@Monika-Bitfly
fix: fix isInvalidItx func
44a6489
@Monika-Bitfly
fix: update isInvalidItx func
848da84
@Monika-Bitfly
test: add tests for isValidERC721Log & isValidERC1155Log methods
6acf6a2
@Monika-Bitfly
fix: update isValidERC721Log func
f20a36a
@Tangui-Bitfly
refactor(execution layer): extract el indexing it own packages
fc7de46
@Tangui-Bitfly
chore: contract simplification
2172697
@Tangui-Bitfly
feat(database transaction): add transaction to database package
29c46c7
@Tangui-Bitfly
fix: rename database.Transaction to database.Batch
b97f7d2
@Tangui-Bitfly
fix: check max uncles len
9bec588
@Tangui-Bitfly
refactor(indexer): remove even more dependancies with database
3380e72
@Tangui-Bitfly
add test for erc20 transform
6c491cd
@Tangui-Bitfly
final transformer refactor
c991ce3
@Tangui-Bitfly
add more tests
79e8597 
fix
@Monika-Bitfly
test: add tests for isValidItx, getTokenID & getContractAddress methods
5d15272
@Monika-Bitfly
test: add tests for TestGetERC1155TransferIDs, TestGetERC1155Transfer…
da3daa1 
…Values & TestCalculateTxFee methods
@Tangui-Bitfly
more tests
a63ebab
Monika-Bitfly and others added 20 commits February 4, 2025 16:13
@Monika-Bitfly
Merge branch 'refactor-indexing' of github.com:gobitfly/beaconchain i…
8f8d4e8 
…nto m/refactor-indexing-v2
@Monika-Bitfly
refactor: update isBlobTx func
b2e9786
@Monika-Bitfly
test: add tests for calculateMevFromBlock, calculateBlockUncleReward,…
4f52647 
… calculateUncleReward & verifyName methods
@Monika-Bitfly
test: add rpc/utils tests
6afef03
@Tangui-Bitfly
fix missing chainID
6e2773f
@Tangui-Bitfly
simplify pr
a5de2d4
@Monika-Bitfly
refactor: update parseAddressBalance function
e1d61ab
@Monika-Bitfly
test: add tests for TestGetTokens & TestParseAddressBalance methods
ced4961
@Tangui-Bitfly
comments
5a9c002 
AdaptorV1
data.Store
metadataupdates.Store
TransformFunc
@Monika-Bitfly
test: add function-level comments to tests
96d9352
@Monika-Bitfly
test: add tests for getBlockUncles method
dbd9038
@Tangui-Bitfly
remove unused func
1a39ae4
@Monika-Bitfly
refactor: updated ERC20 contract handlers name
1bfad7f
@Tangui-Bitfly
fix merge map
b8ad60b
@Monika-Bitfly
test: add tests for getERC20ContractSymbol, getERC20ContractTotalSupp…
6d4a117 
…ly & getERC20ContractDecimals method
@Monika-Bitfly
Merge branch 'refactor-indexing' of github.com:gobitfly/beaconchain i…
353a2f8 
…nto m/refactor-indexing-v2
@Monika-Bitfly
refactor: update getTxRecipient func
6e067ee
@Monika-Bitfly
refactor: update ERC20 contract test func name
eb469a5
@Monika-Bitfly
fix: lint
5c8d3c6
@Monika-Bitfly
test: add rpc/erigon tests
afb5116
@Monika-Bitfly Monika-Bitfly marked this pull request as ready for review February 5, 2025 18:55
Tangui-Bitfly
Tangui-Bitfly reviewed Feb 7, 2025
View reviewed changes
Copy link
Contributor

@Tangui-Bitfly Tangui-Bitfly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a lot of good things in that pr but I'm wondering if some changes go maybe too far (too many little functions, too many tests that test not exported func without direct link to usage, ...) and will be a burden in the long run.

backend/internal/th/ethereum.go Outdated Show resolved Hide resolved
backend/pkg/commons/rpc/erigon.go Outdated Show resolved Hide resolved
backend/pkg/commons/rpc/erigon.go Outdated Show resolved Hide resolved
backend/pkg/commons/rpc/erigon_test.go Show resolved Hide resolved
backend/pkg/commons/rpc/erigon_test.go

// TestGetMaxFeePerBlobGas tests the getMaxFeePerBlobGas function
// which extracts the max fee per blob gas from a transaction
func TestGetMaxFeePerBlobGas(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to test each individual function like that ? Too much tests is like too much codes, harder to maintain, more complex and prone to brake.
I mean what this test is really testing ? Following this logic we could do a test for each field of a block. IMO it's better to create one test where we test each field all together.

backend/pkg/commons/rpc/utils_test.go Show resolved Hide resolved
backend/pkg/commons/rpc/utils_test.go Outdated Show resolved Hide resolved
backend/pkg/executionlayer/transformer.go Outdated Show resolved Hide resolved
backend/pkg/executionlayer/transformer.go Outdated Show resolved Hide resolved
backend/pkg/executionlayer/transformer_test.go Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tangui-Bitfly Tangui-Bitfly Tangui-Bitfly left review comments

@mccreedy-bitfly mccreedy-bitfly Awaiting requested review from mccreedy-bitfly

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Monika-Bitfly @Tangui-Bitfly