fix(provider): resolve bids from sibling wallets at a shared hostUri#3232
Open
baktun14 wants to merge 5 commits into
Open
fix(provider): resolve bids from sibling wallets at a shared hostUri#3232baktun14 wants to merge 5 commits into
baktun14 wants to merge 5 commits into
Conversation
Multiple Provider rows can share the same hostUri on-chain (different owner wallets registering the same daemon). The API dedupes by hostUri and only returns one canonical row per hostUri, so bids submitted from non-canonical wallets at the same hostUri previously failed the `providers.find(p => p.owner === bid.provider)` lookup in the UI — leaving empty rows in the bid list, breaking the search/audited filters, and throwing "Cannot find bid provider" on manifest send. API: - Strengthen the dedup tiebreaker to prefer the wallet with active leases (via new `LeaseRepository.getActiveLeaseCountByProviders`). Falls back to `createdHeight DESC` only when leases are tied. - Expose the sibling wallets that share each canonical row's hostUri as `aliasOwners: string[]` on the provider list response. deploy-web: - Add `findProviderForBidProvider(providers, bidProvider)` helper that resolves a bid by `owner` OR by `aliasOwners`. - Use the helper in BidGroup display + CreateLease manifest send, search filter, and audited filter. - Hide bid rows whose resolved provider is offline or on an invalid version so users no longer see unselectable empty rows on mainnet.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds provider aliasing: LeaseRepository counts active leases per provider owner; ProviderService selects a representative per hostUri using online status, lease counts, and createdHeight; aliasOwners are returned in provider list responses; frontend resolves bids by owner or aliasOwners and uses the lookup in components and tests. ChangesProvider Aliasing Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3232 +/- ##
==========================================
- Coverage 66.69% 65.54% -1.15%
==========================================
Files 1065 981 -84
Lines 26123 24091 -2032
Branches 6291 5883 -408
==========================================
- Hits 17423 15791 -1632
+ Misses 7602 7241 -361
+ Partials 1098 1059 -39
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
Adds the new `aliasOwners` field (and its required-list entry) to the provider-list response in the docs snapshot.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.tsx (1)
263-265:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFavorites filter still uses raw bid owner instead of resolved provider.
Line 264 compares
favoriteProvidersagainstbid.provider; with alias bids, this can hide bids from favorited canonical providers.Proposed fix
if (isFilteringFavorites) { - filteredBids = filteredBids.filter(bid => favoriteProviders.some(y => y === bid.provider)); + filteredBids = filteredBids.filter(bid => { + const provider = findProviderForBidProvider(providers, bid.provider); + return !!provider && favoriteProviders.includes(provider.owner); + }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.tsx` around lines 263 - 265, The favorites filter is comparing favoriteProviders to the raw bid.provider which misses alias bids; update the filter inside the isFilteringFavorites block so it compares favoriteProviders against the bid's resolved/canonical provider identifier used elsewhere (replace bid.provider with the resolved provider field used in the codebase, e.g., bid.resolvedProvider or bid.canonicalProvider), ensuring alias-to-canonical resolution is checked when building filteredBids.
🧹 Nitpick comments (1)
apps/api/src/provider/services/provider/provider.service.spec.ts (1)
405-417: ⚡ Quick winReplace
as unknown as Providercasts with typed mocks/builders in these new tests.The new cases still rely on double-casts, which bypasses type safety and conflicts with your test mocking guideline. Prefer
mock<Provider>()/MockProxy<Provider>and assign only required fields for each scenario.As per coding guidelines "Use vitest-mock-extended with
mock()andMockProxy<T>for mocking in tests" and "Usemock<T>()instead ofas unknown as <Type>for type casting in tests".Also applies to: 463-479
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/provider/services/provider/provider.service.spec.ts` around lines 405 - 417, Replace the double-cast patterns used for test fixtures (e.g., olderOnlineWithLeases and newerOnlineNoLeases created from createProviderWithAttributeSignatures(AUDITOR)) with proper typed mocks using vitest-mock-extended: create mock<Provider>() or MockProxy<Provider> instances, then set only the required properties (hostUri, isOnline, createdHeight and any signature attributes produced by createProviderWithAttributeSignatures) on those mocks instead of using "as unknown as Provider"; update the other occurrences (the block around lines ~463-479) the same way so the tests use type-safe mocks rather than bypassing type checking.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/provider/services/provider/provider.service.ts`:
- Around line 164-167: ownersByHostUri is collecting duplicate owner entries per
provider.hostUri which causes aliasOwners to contain repeats; change the map
value to a Set (e.g., ownersByHostUri: Map<string, Set<string>>) and when
iterating use ownersByHostUri.get(provider.hostUri)?.add(provider.owner) instead
of pushing to an array, then when building aliasOwners convert each Set to an
array (or dedupe there) so aliasOwners for each hostUri is stable and unique;
apply the same change at the other occurrence referenced around lines 183-184.
---
Outside diff comments:
In `@apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.tsx`:
- Around line 263-265: The favorites filter is comparing favoriteProviders to
the raw bid.provider which misses alias bids; update the filter inside the
isFilteringFavorites block so it compares favoriteProviders against the bid's
resolved/canonical provider identifier used elsewhere (replace bid.provider with
the resolved provider field used in the codebase, e.g., bid.resolvedProvider or
bid.canonicalProvider), ensuring alias-to-canonical resolution is checked when
building filteredBids.
---
Nitpick comments:
In `@apps/api/src/provider/services/provider/provider.service.spec.ts`:
- Around line 405-417: Replace the double-cast patterns used for test fixtures
(e.g., olderOnlineWithLeases and newerOnlineNoLeases created from
createProviderWithAttributeSignatures(AUDITOR)) with proper typed mocks using
vitest-mock-extended: create mock<Provider>() or MockProxy<Provider> instances,
then set only the required properties (hostUri, isOnline, createdHeight and any
signature attributes produced by createProviderWithAttributeSignatures) on those
mocks instead of using "as unknown as Provider"; update the other occurrences
(the block around lines ~463-479) the same way so the tests use type-safe mocks
rather than bypassing type checking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 92d37941-07ce-418d-8857-c0a2d93ea141
📒 Files selected for processing (12)
apps/api/src/deployment/repositories/lease/lease.repository.tsapps/api/src/provider/http-schemas/provider.schema.tsapps/api/src/provider/services/provider/provider.service.spec.tsapps/api/src/provider/services/provider/provider.service.tsapps/api/src/types/provider.tsapps/api/src/utils/map/provider.tsapps/deploy-web/src/components/new-deployment/BidGroup.tsxapps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.tsxapps/deploy-web/src/types/provider.tsapps/deploy-web/src/utils/providerUtils.spec.tsapps/deploy-web/src/utils/providerUtils.tsapps/deploy-web/tests/seeders/provider.ts
Contributor
|
This is non-portable hack. It cannot be solved in the same way in provider-intventory. |
Use Set<string> for owners-per-hostUri accumulator so any duplicate provider rows don't produce repeated entries in aliasOwners.
…efer-active-leases
Add unit tests for the search and audited filters in CreateLease that exercise findProviderForBidProvider, including resolving a bid submitted from a sibling wallet via aliasOwners. Raises deploy-web patch coverage of the changed lines above the 50% codecov target (was 40%).
Contributor
|
Actionable comments posted: 0 |
Contributor
Author
Any idea what to do then? |
Contributor
@baktun14 let's get back at the point when the initial dedup logic was introduced and question whether it's needed. If provider sends a bid, it means it's alive, maybe we don't need to dedup anything at all? |
Contributor
Author
It's needed in a few places like the list of providers and the bid list, but I think we can come up with something different and check wallet activity to see which wallet is the good one. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Multiple Provider rows can share the same hostUri on-chain (different owner wallets registering the same daemon — e.g. an operator who rotated wallets but didn't deregister the old ones). The API dedupes by hostUri and only returns one canonical wallet per hostUri, so a bid submitted from a non-canonical sibling wallet failed the
providers.find(p => p.owner === bid.provider)lookup in deploy-web — leaving empty rows in the bid list, breaking the search/audited filters, and throwingCannot find bid provideron manifest send.Fixes CON-375.
Surfaced via a community report (provider with 3 wallets at
provider.cmolls.de:8443, the bidding wallet wasn't the dedup-winning one in our DB).What
API (apps/api):
LeaseRepository.getActiveLeaseCountByProviders(owners)— single grouped query against the existing(providerAddress, closedHeight, createdHeight)index.ProviderService.getProviderList:isOnlinefirst, then active lease count DESC (new), thencreatedHeight DESCas a last resort. Picks the wallet that's actually serving when there's a real signal.aliasOwners: string[]field on the provider list response — the sibling wallets that share the canonical row's hostUri (excluding the canonical owner).deploy-web:
findProviderForBidProvider(providers, bidProvider)helper — resolves a bid byownerOR byaliasOwners.BidGroup(display) andCreateLease(manifest send, search filter, audited filter).Why aliasOwners resolves it even when all sibling wallets have 0 active leases: the dedup just picks a representative per hostUri. Whichever wallet wins, the others land in
aliasOwners. The UI resolves bids from any sibling against the canonical row, and manifest send routes by hostUri (or by the bid's raw owner viafindActiveByAddress, which doesn't dedupe), so the actual chain identity stays attached for on-chain operations.Follow-ups (separate Linear issues, not in this PR)
Companion PR
Same UI changes mirrored to console-air: akash-network/console-air#25 — depends on this PR's API change being deployed before alias resolution does anything useful.
Test plan
npm testinapps/api(provider area — 49/49)npm testinapps/deploy-web(utils + new-deployment — 227/227)npx tsc --noEmitclean for changed files in both appsnpm run lint -- --quietclean for changed files/v1/providersreturnsaliasOwnersand a bid from a sibling wallet at a multi-wallet hostUri now renders and can be selected end-to-end (manifest send works).Summary by CodeRabbit
New Features
Improvements
Tests