fix(satp-hermes): fix ClaimFormat check, BesuLeaf contractName, and OracleEVM class refs#4376
fix(satp-hermes): fix ClaimFormat check, BesuLeaf contractName, and OracleEVM class refs#4376mskrzypkows wants to merge 6 commits into
Conversation
48bd949 to
df72992
Compare
There was a problem hiding this comment.
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
inchecks withArray.prototype.includes()in bridge/oracle execution layers, with added unit tests. - Fix BesuLeaf contract invocation metadata by passing
wrapperContractName(instead ofwrapperContractAddress) ascontractName. - Correct OracleEVM/EthereumLeaf class name references in IDs and error messages, and export
IEthereumNetworkConfigviapublic-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. |
b8e7bf4 to
e6051c0
Compare
…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>
e3bdcb9 to
8a73a58
Compare
…-hermes-fix Signed-off-by: Maciej Skrzypkowski <mskr@gmx.com>
RafaelAPB
left a comment
There was a problem hiding this comment.
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>
1d67a6f to
0e49996
Compare
Three bug fixes and one refactor in the satp-hermes plugin.
Fix
Array.includes()for ClaimFormat validationThe
inoperator checks array indices, not values. SinceClaimFormat.DEFAULT = 1,1 in [1]isfalse(only index0exists), causing every transfer to throwClaimFormatError. BothSATPBridgeExecutionLayerImplandOracleExecutionLayerconstructors are updated to use
Array.prototype.includes(). Unit tests added forthe supported, default-fallback, unsupported, and empty-list cases.
Fix BesuLeaf
contractNamebuglockAsset(),unlockAsset(), andrunTransaction()were passingwrapperContractAddressascontractName, causing an ABI cache miss on every call.Fixed to use
wrapperContractName.Fix OracleEVM and EthereumLeaf class name references
OracleEVMusedEthereumLeaf.CLASS_NAMEforthis.idand in aNoSigningCredentialErrormessage — both replaced withOracleEVM.CLASS_NAME.The
ConnectorOptionsErrorinEthereumLeaf's constructor incorrectly said"FabricLeaf"; corrected to"EthereumLeaf". Removed the now-unusedEthereumLeafimport fromOracleEVM.Refactor: export
IEthereumNetworkConfigExports the configuration interface that consumers need to connect gateways to
Ethereum networks.