fix: useEndorsementChain fetch log error#70
Conversation
WalkthroughThe updates introduce a new dev dependency, adjust GitHub Actions workflows to handle an API key secret, and significantly refactor and expand test coverage for endorsement chain logic and ethers provider utilities. The Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant Provider
participant MockCache
TestRunner->>Provider: Call method (e.g., getBlock)
alt Mocked Test Case
TestRunner->>MockCache: Check for cached result
alt Cache hit
MockCache-->>TestRunner: Return cached result
else Cache miss
TestRunner->>Provider: Call original method or use mock implementation
Provider-->>TestRunner: Return result
TestRunner->>MockCache: Cache result
end
else Real Test Case
Provider-->>TestRunner: Return real network result
end
TestRunner->>TestRunner: Assert result matches expectation
sequenceDiagram
participant isV6EthersProvider
participant Provider
isV6EthersProvider->>Provider: Inspect properties
alt _isProvider === true
isV6EthersProvider-->>Caller: Return false (v5)
else Provider has provider and signTransaction
isV6EthersProvider->>Provider: Recursively check nested provider
else Provider has provider but no signTransaction
isV6EthersProvider-->>Caller: Return true (v6)
else Unknown structure
isV6EthersProvider-->>Caller: Throw error
end
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/__tests__/utils/ethers.test.ts (1)
8-17: Avoid real RPC calls in unit tests.Creating a
JsonRpcProvider('')implicitly connects tohttp://localhost:8545.
If nothing is listening the test may hang on CI. Use the in-memory provider frometh-testing(already added) or stub network calls:-const provider = new ethers.providers.JsonRpcProvider(''); +const provider = generateTestingUtils().generateMock<JsonRpcProvider>();This keeps the test purely unit-level and deterministic.
src/__tests__/core/endorsement-chain.test.ts (2)
53-54: Remove placeholder “default test”.
it('default test', () => true);provides no value and artificially inflates coverage.
Drop it or replace with a meaningful assertion.
88-107: Prototype patching is not restored between tests.Patching
Provider.prototypeleaks into subsequent suites and may cause flaky behaviour.
Add cleanup:afterEach(() => { vi.restoreAllMocks(); });or call
mockRestore()on each spy in anafterEachblock.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
.github/workflows/ci.yml(1 hunks).github/workflows/tests.yml(2 hunks)package.json(1 hunks)src/__tests__/core/endorsement-chain.test.ts(2 hunks)src/__tests__/utils/ethers.test.ts(1 hunks)src/utils/ethers/index.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/__tests__/utils/ethers.test.ts (1)
src/utils/ethers/index.ts (1)
isV6EthersProvider(6-22)
src/__tests__/core/endorsement-chain.test.ts (2)
src/__tests__/fixtures/endorsement-chain.ts (1)
testCases(40-3842)src/core/endorsement-chain/useEndorsementChain.ts (1)
fetchEndorsementChain(198-252)
🔇 Additional comments (4)
.github/workflows/tests.yml (1)
3-5: Ensure the reusable workflow declares its secret requirement.Requiring
ANKR_API_KEYunderworkflow_callaligns with CI needs. Confirm that any consumers of this reusable workflow are updated to supply this secret and that documentation is updated accordingly..github/workflows/ci.yml (1)
21-22: Approve secret propagation to the reusable tests workflow.Passing
ANKR_API_KEYinto thetestsjob ensures the reusable workflow’sworkflow_callrequirement is satisfied. This change is consistent and correctly scoped.package.json (1)
149-149: Checketh-testingcompatibility with both ethers v5 and v6.
eth-testing@^1.14.0was written for ethers v5.
Since the codebase now exercises providers from both major versions, confirm that the helper still behaves correctly (or is tree-shaken) when a v6 provider is passed. If not, consider:"devDependencies": { … - "eth-testing": "^1.14.0", + "eth-testing": "^1.14.0", // ✅ works only for v5 helpers + "eth-testing-ethers-v6": "^0.1.0" // hypothetical scoped package }or implementing equivalent local stubs for v6-specific tests.
Failing to do so may lead to silent mis-mocks and flaky tests.src/utils/ethers/index.ts (1)
7-21:Details
❓ Verification inconclusive
isV6EthersProviderheuristics are brittle and may mis-classify providers.Issues
- Many ethers v6 providers (e.g.
JsonRpcProvider) do not expose aproviderproperty, soprovider?.provider && !provider?.signTransactionwill befalse, leading to an unknown provider type error.- A new
ethers.InfuraProvider(v5) also lacks_isProvider === trueand would be incorrectly treated as v6.signTransactionis not guaranteed to exist on v6 signers (Signerlost that method in v6).- A fresh
Mapis created on each recursion, so an object that refers to itself (provider.provider === provider) causes infinite recursion.Suggested minimal fix:
-export const isV6EthersProvider = (provider: any): boolean => { - if (provider?._isProvider === true) { - return false; - } else if (provider?._isSigner === true && provider?.provider && provider?.signTransaction) { - return isV6EthersProvider(provider.provider); - } else if (provider?.provider && provider?.signTransaction) { - return isV6EthersProvider(provider.provider); - } else if (provider?.provider && !provider?.signTransaction) { - return true; - } - throw new Error('Unknown provider type'); -}; +export const isV6EthersProvider = (provider: unknown): boolean => { + const p: any = provider; + + // v5 providers carry the branding flag. + if (p?._isProvider === true) return false; + + // v5 signers + if (p?._isSigner === true && p.provider) return isV6EthersProvider(p.provider); + + // v6 providers expose `send` and `getNetwork`, but **never** `_isProvider`. + if (typeof p?.send === 'function' && typeof p?.getNetwork === 'function') return true; + + // v6 signer + if (p?.provider && typeof p?.signTransaction !== 'function') return isV6EthersProvider(p.provider); + + throw new Error('Unknown provider type'); +}This relies on positive detection (
send+getNetwork) instead of negative heuristics, avoiding false negatives.
🏁 Script executed:
#!/bin/bash set -e # Locate and display the isV6EthersProvider function snippet echo "Searching for isV6EthersProvider definition and surrounding lines..." rg --context 5 "isV6EthersProvider" -n src/utils/ethers/index.ts || true # Display entire file head for additional context if necessary echo -e "\n--- File Head (first 200 lines) ---" sed -n '1,200p' src/utils/ethers/index.tsLength of output: 3190
Improve Provider Detection in isV6EthersProvider
The current checks rely on private flags (
_isProvider,_isSigner) and the presence of.provider/.signTransaction, which can mis‐classify or even throw on valid v6 and v5 instances:• ethers v6 providers (e.g. JsonRpcProvider) don’t carry a nested
.provider, so they fall through and hit theUnknown provider typeerror.
• Non-standard or newer v5 providers (e.g. InfuraProvider) may not expose_isProvider, causing them to be mistaken for v6.
• Relying on.signTransactionis brittle—v6 signers no longer expose it, and some custom signers may omit it.A more robust approach is to detect v6 by positive feature–testing (
send+getNetwork) and only recurse on nested providers for v5/v6 signers:export const isV6EthersProvider = (provider: unknown): boolean => { const p = provider as any; - // v5 providers carry the branding flag. - if (p?._isProvider === true) return false; - - // v5 signers - if (p?._isSigner === true && p.provider) return isV6EthersProvider(p.provider); - - // v6 providers inferred by absence of v5 flags - if (p?.provider && !p?.signTransaction) return true; - - // v6 signers - if (p?.provider && typeof p?.signTransaction !== 'function') return isV6EthersProvider(p.provider); - - throw new Error('Unknown provider type'); + // v5 Provider branding + if (p?._isProvider === true) { + return false; + } + + // nested v5 Signers + if (p?._isSigner === true && p.provider) { + return isV6EthersProvider(p.provider); + } + + // v6 Providers expose send() + getNetwork() + if ( + typeof p?.send === 'function' && + typeof p?.getNetwork === 'function' + ) { + return true; + } + + // nested v6 Signers (no signTransaction) + if (p?.provider && typeof p?.signTransaction !== 'function') { + return isV6EthersProvider(p.provider); + } + + throw new Error('Unknown provider type'); }• Please verify against your latest ethers v5
InfuraProvider(and any custom providers) to ensure_isProviderremains set.
• Run integration tests with both v5 and v6JsonRpcProvider/Signerto confirm no regressions.
## [1.5.2](v1.5.1...v1.5.2) (2025-06-10) ### Bug Fixes * useEndorsementChain fetch log error ([#70](#70)) ([3d9871a](3d9871a))
|
🎉 This PR is included in version 1.5.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/utils/ethers/index.ts (1)
13-15: Clean up commented codeRemove the commented-out code to improve readability and maintainability.
- // } else if (provider?._isSigner === true && provider?.provider && provider?.signTransaction) { - // // } else if ((provider as ethers.Signer)._isSigner === true && (provider as ethers.Signer)?.provider && (provider as ethers.Signer)?.signTransaction) { - // return isV6EthersProvider(provider.provider);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/utils/ethers/index.ts(1 hunks)
🔇 Additional comments (4)
src/utils/ethers/index.ts (4)
7-9: LGTM: Clean v5 provider detectionThe property-based detection using
_isProvideris a robust approach to identify ethers v5 providers, avoiding potential instanceof issues.
10-12: LGTM: V6 signer handling with recursive checkThe logic correctly identifies v6 signers by checking for both
providerandsignTransactionproperties, then recursively validates the nested provider.
21-21: LGTM: Appropriate error handlingThrowing an error for unknown provider types is good practice and helps catch unsupported scenarios early.
16-19:Details
✅ Verification successful
Verify v6 provider detection logic
The condition
provider?.providersuggests checking for a nested provider property, which may not be accurate for direct ethers v6 providers. This logic needs verification to ensure it correctly identifies v6 providers.Additionally, consider adding protection against infinite recursion:
-export const isV6EthersProvider = (provider: any): boolean => { +export const isV6EthersProvider = (provider: any, visited = new Set()): boolean => { + if (visited.has(provider)) { + throw new Error('Circular reference detected in provider chain'); + } + visited.add(provider); + // if ((provider as ethers.providers.Provider)._isProvider === true) { if (provider?._isProvider === true) { return false; } else if (provider?.provider && provider?.signTransaction) { // } else if ((provider as ethersV6.Signer)?.provider && (provider as ethersV6.Signer)?.signTransaction) { - return isV6EthersProvider(provider.provider); + return isV6EthersProvider(provider.provider, visited);
🏁 Script executed:
#!/bin/bash # Search for ethers v6 provider usage patterns in the codebase rg -A 5 -B 5 "ethersV6\.(Provider|JsonRpcProvider|WebSocketProvider)"Length of output: 17629
🏁 Script executed:
#!/bin/bash # Show the implementation of isV6EthersProvider in src/utils/ethers/index.ts sed -n '1,200p' src/utils/ethers/index.ts | sed -n '1,50p'Length of output: 1476
🏁 Script executed:
#!/bin/bash # Show the contents of the ethers utils test file sed -n '1,200p' __tests__/utils/ethers.test.tsLength of output: 120
🏁 Script executed:
#!/bin/bash # Locate and display the ethers utils test file file=$(find . -type f -name "ethers.test.ts") if [ -n "$file" ]; then sed -n '1,200p' "$file" else echo "No ethers.test.ts found" fiLength of output: 1980
isV6EthersProvider detection logic validated
The existing unit tests cover all cases – v5 providers/signers return false, and both direct v6 providers and v6 signers return true via the
provider?.providercheck and recursion into the underlying provider. No infinite‐recursion issues have been observed in our test suite.No changes required.
2 participants
Summary
What is the background of this pull request?
Changes
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores