Skip to content

fix(satp-hermes): fix ClaimFormat check, BesuLeaf contractName, and OracleEVM class refs#4376

Open
mskrzypkows wants to merge 6 commits into
hyperledger-cacti:mainfrom
mskrzypkows:satp-hermes-fix
Open

fix(satp-hermes): fix ClaimFormat check, BesuLeaf contractName, and OracleEVM class refs#4376
mskrzypkows wants to merge 6 commits into
hyperledger-cacti:mainfrom
mskrzypkows:satp-hermes-fix

Conversation

@mskrzypkows

Copy link
Copy Markdown

Three bug fixes and one refactor in the satp-hermes plugin.

Fix Array.includes() for ClaimFormat validation
The in operator checks array indices, not values. Since ClaimFormat.DEFAULT = 1,
1 in [1] is false (only index 0 exists), causing every transfer to throw
ClaimFormatError. Both SATPBridgeExecutionLayerImpl and OracleExecutionLayer
constructors are updated to use Array.prototype.includes(). Unit tests added for
the supported, default-fallback, unsupported, and empty-list cases.

Fix BesuLeaf contractName bug
lockAsset(), unlockAsset(), and runTransaction() were passing
wrapperContractAddress as contractName, causing an ABI cache miss on every call.
Fixed to use wrapperContractName.

Fix OracleEVM and EthereumLeaf class name references
OracleEVM used EthereumLeaf.CLASS_NAME for this.id and in a
NoSigningCredentialError message — both replaced with OracleEVM.CLASS_NAME.
The ConnectorOptionsError in EthereumLeaf's constructor incorrectly said
"FabricLeaf"; corrected to "EthereumLeaf". Removed the now-unused
EthereumLeaf import from OracleEVM.

Refactor: export IEthereumNetworkConfig
Exports the configuration interface that consumers need to connect gateways to
Ethereum networks.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses several SATP Hermes interoperability bugs (claim format validation, Besu wrapper contract invocation metadata, and OracleEVM/EthereumLeaf class-reference strings) and includes a small public API export refactor for Ethereum network config typing.

Changes:

  • Fix claim format validation by replacing incorrect in checks with Array.prototype.includes() in bridge/oracle execution layers, with added unit tests.
  • Fix BesuLeaf contract invocation metadata by passing wrapperContractName (instead of wrapperContractAddress) as contractName.
  • Correct OracleEVM/EthereumLeaf class name references in IDs and error messages, and export IEthereumNetworkConfig via public-api.ts.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/cactus-plugin-satp-hermes/src/test/typescript/unit/claim-format-validation.test.ts Adds unit coverage for claim format validation behaviors (supported/default/unsupported/empty).
packages/cactus-plugin-satp-hermes/src/main/typescript/public-api.ts Exposes IEthereumNetworkConfig for consumers via the package barrel export.
packages/cactus-plugin-satp-hermes/src/main/typescript/cross-chain-mechanisms/oracle/oracle-execution-layer.ts Fixes claim-format validation logic to use includes().
packages/cactus-plugin-satp-hermes/src/main/typescript/cross-chain-mechanisms/oracle/implementations/oracle-evm.ts Fixes OracleEVM class-name references and removes unused import.
packages/cactus-plugin-satp-hermes/src/main/typescript/cross-chain-mechanisms/bridge/satp-bridge-execution-layer-implementation.ts Fixes claim-format validation logic to use includes().
packages/cactus-plugin-satp-hermes/src/main/typescript/cross-chain-mechanisms/bridge/leafs/ethereum-leaf.ts Corrects constructor error message to reference EthereumLeaf (not FabricLeaf).
packages/cactus-plugin-satp-hermes/src/main/typescript/cross-chain-mechanisms/bridge/leafs/besu-leaf.ts Fixes contract invocation contractName to use wrapperContractName to avoid ABI cache misses.

mskrzypkows and others added 4 commits June 15, 2026 09:53
…references

BesuLeaf passed wrapperContractAddress as contractName in lockAsset(), unlockAsset(),
and runTransaction(), causing an ABI cache miss on every call. Fixed to use
wrapperContractName.

ConnectorOptionsError in EthereumLeaf constructor said "FabricLeaf" — corrected to
"EthereumLeaf".

OracleEVM used EthereumLeaf.CLASS_NAME for this.id assignment and in a
NoSigningCredentialError message. Both replaced with OracleEVM.CLASS_NAME.
Removed the now-unused EthereumLeaf import.

Assisted-by: anthropic:claude-sonnet-4.6
Signed-off-by: Maciej Skrzypkowski <mskr@gmx.com>
…t check

The 'in' operator checks for array indices (0, 1, 2...) not values.
ClaimFormat.DEFAULT=1, so '1 in [1]' is false (only index 0 exists),
causing every transfer to throw ClaimFormatError.

Fix both SATPBridgeExecutionLayerImpl and OracleExecutionLayer constructors
to use Array.prototype.includes() which checks values.

Add unit tests covering: supported format accepts, DEFAULT fallback, unsupported
format throws ClaimFormatError, and empty supported list throws ClaimFormatError.

Assisted-by: anthropic:claude-sonnet-4.6
Signed-off-by: Maciej Skrzypkowski <mskr@gmx.com>
the configuration interface consumers need to
connect gateways to Ethereum networks.

Assisted-by: anthropic:claude-sonnet-4.6
Signed-off-by: Maciej Skrzypkowski <mskr@gmx.com>
Signed-off-by: Maciej Skrzypkowski <mskr@gmx.com>
…-hermes-fix

Signed-off-by: Maciej Skrzypkowski <mskr@gmx.com>

@RafaelAPB RafaelAPB left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Please add the required test and we are good to go! (you may add a few assertions in tests that do contract invocations to test the contract name parameter)

@RafaelAPB

Copy link
Copy Markdown
Contributor

LGTM. Please add the required test and we are good to go! (you may add a few assertions in tests that do contract invocations to test the contract name parameter)

@mskrzypkows thank you for adding the test; however, instead of us wasting a lot of Ci resources, we can test your fix with a couple assertions in other besu tests. Can you please make that change?

Signed-off-by: Maciej Skrzypkowski <mskr@gmx.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants