[KLC-1409] Update integration tests with Klever Blockchain Mock#12
Conversation
WalkthroughThe codebase was refactored to replace all references to the MultiversX blockchain with the Klever blockchain, affecting mock implementations, configuration keys, file naming conventions, and method/type names throughout integration tests and factory components. New mock files for Klever blockchain were introduced, while MultiversX-specific mocks were removed. No core logic or control flow was altered. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Integration Test
participant KleverMock as KleverBlockchainMock
participant AccountsMock as KleverAccountsMock
Test->>KleverMock: SendTransaction/SendTransactions
KleverMock->>AccountsMock: updateNonce(address, nonce)
Test->>KleverMock: ExecuteVMQuery
KleverMock-->>Test: Return VM query response
Test->>KleverMock: GetAccount(address)
KleverMock-->>Test: Return mock account
Poem
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
integrationTests/mock/KleverContractStateMock.go (1)
492-503: 🛠️ Refactor suggestionShadowing the imported
addresspackage and discarding errors hampers readability
addressvariable shadows the imported package name, and the error fromNewAddressFromBytesis only logged via panic.
Return a NOK VM response instead of crashing, and rename the local to avoid confusion:- addressBytes, err := hex.DecodeString(hexAddress) + addrBytes, err := hex.DecodeString(hexAddress) - address, err := address.NewAddressFromBytes(addressBytes) + addr, err := address.NewAddressFromBytes(addrBytes) if err != nil { - panic(err) + return createNokVmResponse(err) } - bech32Address := address.Bech32() + bech32Address := addr.Bech32()
🧹 Nitpick comments (11)
kleverchain.workspace.json (1)
1-1: Document or remove empty workspace file
This file currently contains only{}; please clarify its intended content or usage in project workflows or remove it if it's unnecessary as a placeholder.integrationTests/relayers/ethToKleverBlockchain_test.go (3)
103-103: Refactor variable name for clarity
The mock instantiation is assigned tomultiversXChainMockdespite usingNewKleverBlockchainMock(). Rename the variable tokleverChainMock(and update its references) to avoid confusion.
319-319: Consistent mock variable naming in second test
Similarly, changemultiversXChainMock := mock.NewKleverBlockchainMock()tokleverChainMock := mock.NewKleverBlockchainMock()and update all downstream uses.
399-400: Update helper function parameter name
IncreateMockBridgeComponentsArgs, the parametermultiversXChainMock *mock.KleverBlockchainMockshould be renamed tokleverChainMockto match the new mock type and avoid mixing legacy naming.integrationTests/mock/KleverAccountsMock.go (1)
29-32: updateNonce should populate the Account if it was just created
getOrCreatereturns a blankmodels.Account{}.
Consider at least storing the address string so further assertions can rely on it:if !found { acc = &models.Account{} mock.accounts[addrAsString] = acc } +acc.Address = addrAsStringintegrationTests/relayers/kleverBlockchainToEth_test.go (3)
34-41: Rename test to reflect Klever instead of MultiversXAfter the migration the test name and helper still contain “MultiversX”.
Keeping old names is confusing and may mislead grep-based tooling.-func TestRelayersShouldExecuteSimpleTransfersFromMultiversXToEth(t *testing.T) { +func TestRelayersShouldExecuteSimpleTransfersFromKleverToEth(t *testing.T) {Apply similar renames for the second test and helper functions.
61-69: Variable name no longer matches the underlying typemultiversXChainMock := mock.NewKleverBlockchainMock()Using
multiversXChainMockfor a Klever mock breaks readability and may cause accidental mix-ups later.
Rename toklvChainMock(or similar) throughout the file.
249-257: Use Klever address generator for deposit source
testsCommon.CreateRandomMultiversXAddress()survived the rename.
Switch to a Klever-specific helper (or reuse a generic one) to avoid semantic confusion.-From: testsCommon.CreateRandomMultiversXAddress(), +From: testsCommon.CreateRandomKleverAddress(),integrationTests/mock/KleverChainMock.go (1)
64-69: Prefer read lock for pure read operation
GetAccountacquires a full write lock (Lock) even though it only reads from the map aftergetOrCreate.
UseRLockto improve concurrency –getOrCreatealready handles mutation internally.integrationTests/mock/KleverContractStateMock.go (2)
157-163: Avoid per-call Blake2b hasher allocation
factoryHasher.NewHasher("blake2b")is relatively expensive and is invoked on every status / transfer creation.
Cache the hasher once at struct level (or usesync.Once) to improve test-suite latency.-type kleverBlockchainContractStateMock struct { +type kleverBlockchainContractStateMock struct { *tokensRegistryMock + hasher hashing.Hasher ... } func newKleverBlockchainContractStateMock() *kleverBlockchainContractStateMock { mock := &kleverBlockchainContractStateMock{ tokensRegistryMock: &tokensRegistryMock{}, + hasher: must(factoryHasher.NewHasher("blake2b")), }(Use a small helper
must()or inline error handling.)Also applies to: 220-226
215-218: Minor: unnecessary temporaryindexIncrementValue
indexIncrementValueis constant (6) and only used once:- indexIncrementValue := 6 - ... - currentIndex += indexIncrementValue + currentIndex += 6
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
integrationTests/relayers/testdata/multiversx0.pemis excluded by!**/*.pemintegrationTests/relayers/testdata/multiversx1.pemis excluded by!**/*.pemintegrationTests/relayers/testdata/multiversx2.pemis excluded by!**/*.pem
📒 Files selected for processing (11)
factory/ethKleverBridgeComponents.go(3 hunks)integrationTests/mock/KleverAccountsMock.go(1 hunks)integrationTests/mock/KleverChainMock.go(1 hunks)integrationTests/mock/KleverContractStateMock.go(24 hunks)integrationTests/mock/multiversXAccountsMock.go(0 hunks)integrationTests/mock/multiversXChainMock.go(0 hunks)integrationTests/relayers/common.go(2 hunks)integrationTests/relayers/ethToKleverBlockchain_test.go(4 hunks)integrationTests/relayers/kleverBlockchainToEth_test.go(7 hunks)integrationTests/relayers/slowTests/framework/keys.go(1 hunks)kleverchain.workspace.json(1 hunks)
💤 Files with no reviewable changes (2)
- integrationTests/mock/multiversXAccountsMock.go
- integrationTests/mock/multiversXChainMock.go
🧰 Additional context used
🧬 Code Graph Analysis (4)
integrationTests/relayers/ethToKleverBlockchain_test.go (1)
integrationTests/mock/KleverChainMock.go (2)
NewKleverBlockchainMock(34-40)KleverBlockchainMock(26-31)
integrationTests/mock/KleverAccountsMock.go (2)
clients/klever/proxy/models/accounts.go (1)
Account(8-13)clients/klever/blockchain/address/interfaces.go (1)
Address(3-10)
integrationTests/relayers/kleverBlockchainToEth_test.go (5)
integrationTests/mock/KleverChainMock.go (2)
NewKleverBlockchainMock(34-40)KleverBlockchainMock(26-31)integrationTests/mock/KleverContractStateMock.go (2)
KleverBlockchainPendingBatch(40-43)KleverBlockchainDeposit(46-52)clients/klever/blockchain/address/interfaces.go (1)
Address(3-10)testsCommon/addressGenerators.go (1)
CreateRandomEthereumAddress(11-16)integrationTests/mock/ethereumChainMock.go (1)
EthereumChainMock(30-50)
integrationTests/mock/KleverChainMock.go (8)
clients/klever/proxy/models/vmValues.go (2)
VmValueRequest(29-35)VmValuesResponseData(8-10)clients/klever/blockchain/address/interfaces.go (1)
Address(3-10)clients/klever/proxy/models/accounts.go (1)
Account(8-13)clients/klever/proxy/models/transactions.go (1)
KDAFungibleTokenData(50-54)clients/klever/proxy/models/network.go (2)
NetworkConfig(12-19)NodeOverview(32-44)clients/klever/blockchain/address/address.go (1)
NewAddress(59-80)integrationTests/testscommon.go (1)
TestMarshalizer(31-31)integrationTests/mock/KleverContractStateMock.go (2)
KleverBlockchainPendingBatch(40-43)KleverBlockchainDeposit(46-52)
🔇 Additional comments (10)
integrationTests/relayers/slowTests/framework/keys.go (1)
22-22: Rename PEM path format for Klever keys
The update from"multiversx%d.pem"to"klever%d.pem"looks correct. Ensure that any existing test fixture files or scripts producing PEM files are updated to match this new pattern.integrationTests/relayers/common.go (2)
96-96: Align Klever private key file path
Usingfmt.Sprintf("klever%d.pem", index)is consistent with the renamed PEM format. No further action needed unless downstream code still references the old filename.
111-112: Update state machine keys to Klever context
The keys"EthereumToKleverBlockchain"and"KleverBlockchainToEthereum"correctly replace the old MultiversX constants. This matches factory renames and ensures state machines are wired properly.factory/ethKleverBridgeComponents.go (4)
177-181: Invoke renamed bridge creation method
The call tocreateEthereumToKleverBlockchainBridgecorrectly replaces the old MultiversX variant. Ensure no leftover calls tocreateEthereumToMultiversXBridgeremain.
182-186: Invoke renamed state machine creation method
CallingcreateEthereumToKleverBlockchainStateMachineis consistent with the new naming. All related tests and state machine mappings should align with this.
487-555: RenamecreateEthereumToKleverBlockchainBridgefunction signature
The function has been updated to match the new naming. Verify that theStateMachinekeys ("EthereumToKleverBlockchain") correspond to this name in configuration.
674-709: RenamecreateEthereumToKleverBlockchainStateMachinefunction signature
The renamed state machine constructor aligns with the bridge method. Confirm that polling handlers and log identifiers use the updated name.integrationTests/relayers/kleverBlockchainToEth_test.go (1)
117-120: callIsFromBalanceValidator still looks for “PendingMvxBatches”The stack-frame check is now brittle because the function names changed. Update the substring to the Klever equivalent or refactor the validator detection altogether.
integrationTests/mock/KleverChainMock.go (1)
156-163: addTokensPair already locks – avoid double locking
AddTokensPairacquires the outer lock and then callsmock.addTokensPair, which (insidekleverBlockchainContractStateMock) also takes a lock.
Consider exposing an un-locked helper or dropping the outer lock to prevent future deadlocks if lock ordering changes.integrationTests/mock/KleverContractStateMock.go (1)
560-576: InconsistentReturnCodecapitalisation between OK/NOK responses
createOkVmResponsereturns"Ok"(capital “O”) whilecreateNokVmResponsereturns"nok"(lowercase).
If callers perform case-sensitive checks (very common in mocks), this asymmetry causes subtle bugs.- ReturnCode: "Ok", + ReturnCode: "ok",or change the NOK path to
"Nok"– just keep them aligned.
|
I am manually merging this PR because there are some CI errors, but the CI is not set up yet. Also, it is not the purpose of this PR to fix the CI. |
nickgs1337
deleted the
KLC-1409-update-integration-tests-with-klever-chain-mock
branch
2 participants
Summary
This PR aproaches the task purpouse of adjusting existing mocks to match the rest of the project structure. Many files were changed and also applied some renaming in old methods.
This PR does not fixes all the tests, the enabled tests are passing in VSCode, but, some disabled tests still failing. I fixed many issues with the failing tests, but, i did not go any further on the investigation because the error was occuring in bridge logic layer apparently, so to avoid missfixing some test because of a missunderstand of the bridge workflow, i prefer to leave that tests as is to the professionals.
Introduction
This pull request introduces significant changes to transition the codebase from supporting the MultiversX blockchain to supporting the Klever blockchain. The changes include renaming functions, data structures, and files to reflect this shift, as well as adding new mock implementations for integration testing with Klever blockchain components.
Transition to Klever Blockchain Support:
Renaming Functions and Data Structures:
factory/ethKleverBridgeComponents.goto replace references to "MultiversX" with "KleverBlockchain" (e.g.,createEthereumToMultiversXBridge→createEthereumToKleverBlockchainBridge). [1] [2] [3]File and Type Renaming:
integrationTests/mock/multiversXContractStateMock.gotointegrationTests/mock/KleverContractStateMock.goand updated all type names within the file to align with Klever blockchain terminology (e.g.,multiversXProposedStatus→kleverBlockchainProposedStatus). [1] [2] [3]New Mock Implementations for Klever Blockchain:
kleverBlockchainAccountsMock:KleverBlockchainMock:Miscellaneous Updates:
blake2balgorithm. [1] [2]These changes collectively prepare the codebase for integration with the Klever blockchain ecosystem, ensuring compatibility and providing robust testing support.
Summary by CodeRabbit