Skip to content

Commit 8171da4

Browse files
Chaitu-TatipamulaChaitu-Tatipamula
authored
feat: remove provierId from ServiceProviderInfo struct (#247)
# Remove providerId from ServiceProviderInfo struct ## Summary Remove `providerId` from `ServiceProviderInfo` struct and introduce `ServiceProviderInfoView` as a view struct to return provider ID with provider info. ## Related Issue Fixes #234 ## Changes - **Removed** `providerId` field from `ServiceProviderInfo` struct - **Added** `ServiceProviderInfoView` struct with `providerId` field for external consumption - **Updated** all getter functions to return `ServiceProviderInfoView` - **Updated** tests and ABI definitions ## Files Modified - `ServiceProviderRegistry.sol` - `ServiceProviderRegistryStorage.sol` - `FilecoinWarmStorageService.sol` - Test files and ABI
1 parent 9a81882 commit 8171da4

File tree

6 files changed

+129
-107
lines changed

6 files changed

+129
-107
lines changed

‎service_contracts/abi/ServiceProviderRegistry.abi.json‎

Lines changed: 66 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -414,11 +414,6 @@
414414
"name": "isActive",
415415
"type": "bool",
416416
"internalType": "bool"
417-
},
418-
{
419-
"name": "providerId",
420-
"type": "uint256",
421-
"internalType": "uint256"
422417
}
423418
]
424419
},
@@ -695,37 +690,44 @@
695690
{
696691
"name": "info",
697692
"type": "tuple",
698-
"internalType": "struct ServiceProviderRegistryStorage.ServiceProviderInfo",
693+
"internalType": "struct ServiceProviderRegistry.ServiceProviderInfoView",
699694
"components": [
700-
{
701-
"name": "serviceProvider",
702-
"type": "address",
703-
"internalType": "address"
704-
},
705-
{
706-
"name": "payee",
707-
"type": "address",
708-
"internalType": "address"
709-
},
710-
{
711-
"name": "name",
712-
"type": "string",
713-
"internalType": "string"
714-
},
715-
{
716-
"name": "description",
717-
"type": "string",
718-
"internalType": "string"
719-
},
720-
{
721-
"name": "isActive",
722-
"type": "bool",
723-
"internalType": "bool"
724-
},
725695
{
726696
"name": "providerId",
727697
"type": "uint256",
728698
"internalType": "uint256"
699+
},
700+
{
701+
"name": "info",
702+
"type": "tuple",
703+
"internalType": "struct ServiceProviderRegistryStorage.ServiceProviderInfo",
704+
"components": [
705+
{
706+
"name": "serviceProvider",
707+
"type": "address",
708+
"internalType": "address"
709+
},
710+
{
711+
"name": "payee",
712+
"type": "address",
713+
"internalType": "address"
714+
},
715+
{
716+
"name": "name",
717+
"type": "string",
718+
"internalType": "string"
719+
},
720+
{
721+
"name": "description",
722+
"type": "string",
723+
"internalType": "string"
724+
},
725+
{
726+
"name": "isActive",
727+
"type": "bool",
728+
"internalType": "bool"
729+
}
730+
]
729731
}
730732
]
731733
}
@@ -746,37 +748,44 @@
746748
{
747749
"name": "info",
748750
"type": "tuple",
749-
"internalType": "struct ServiceProviderRegistryStorage.ServiceProviderInfo",
751+
"internalType": "struct ServiceProviderRegistry.ServiceProviderInfoView",
750752
"components": [
751-
{
752-
"name": "serviceProvider",
753-
"type": "address",
754-
"internalType": "address"
755-
},
756-
{
757-
"name": "payee",
758-
"type": "address",
759-
"internalType": "address"
760-
},
761-
{
762-
"name": "name",
763-
"type": "string",
764-
"internalType": "string"
765-
},
766-
{
767-
"name": "description",
768-
"type": "string",
769-
"internalType": "string"
770-
},
771-
{
772-
"name": "isActive",
773-
"type": "bool",
774-
"internalType": "bool"
775-
},
776753
{
777754
"name": "providerId",
778755
"type": "uint256",
779756
"internalType": "uint256"
757+
},
758+
{
759+
"name": "info",
760+
"type": "tuple",
761+
"internalType": "struct ServiceProviderRegistryStorage.ServiceProviderInfo",
762+
"components": [
763+
{
764+
"name": "serviceProvider",
765+
"type": "address",
766+
"internalType": "address"
767+
},
768+
{
769+
"name": "payee",
770+
"type": "address",
771+
"internalType": "address"
772+
},
773+
{
774+
"name": "name",
775+
"type": "string",
776+
"internalType": "string"
777+
},
778+
{
779+
"name": "description",
780+
"type": "string",
781+
"internalType": "string"
782+
},
783+
{
784+
"name": "isActive",
785+
"type": "bool",
786+
"internalType": "bool"
787+
}
788+
]
780789
}
781790
]
782791
}
@@ -880,11 +889,6 @@
880889
"name": "isActive",
881890
"type": "bool",
882891
"internalType": "bool"
883-
},
884-
{
885-
"name": "providerId",
886-
"type": "uint256",
887-
"internalType": "uint256"
888892
}
889893
]
890894
},
@@ -1139,11 +1143,6 @@
11391143
"name": "isActive",
11401144
"type": "bool",
11411145
"internalType": "bool"
1142-
},
1143-
{
1144-
"name": "providerId",
1145-
"type": "uint256",
1146-
"internalType": "uint256"
11471146
}
11481147
],
11491148
"stateMutability": "view"

‎service_contracts/src/FilecoinWarmStorageService.sol‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import {Payments, IValidator} from "@fws-payments/Payments.sol";
1515
import {Errors} from "./Errors.sol";
1616

1717
import {ServiceProviderRegistry} from "./ServiceProviderRegistry.sol";
18-
import {ServiceProviderRegistryStorage} from "./ServiceProviderRegistryStorage.sol";
1918

2019
import {Extsload} from "./Extsload.sol";
2120

@@ -490,9 +489,9 @@ contract FilecoinWarmStorageService is
490489
// Check if provider is approved
491490
require(approvedProviders[providerId], Errors.ProviderNotApproved(serviceProvider, providerId));
492491

493-
ServiceProviderRegistryStorage.ServiceProviderInfo memory providerInfo =
492+
ServiceProviderRegistry.ServiceProviderInfoView memory providerInfo =
494493
serviceProviderRegistry.getProvider(providerId);
495-
address payee = providerInfo.payee;
494+
address payee = providerInfo.info.payee;
496495

497496
uint256 clientDataSetId = clientDataSetIds[createData.payer]++;
498497
clientDataSets[createData.payer].push(dataSetId);

‎service_contracts/src/ServiceProviderRegistry.sol‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@ contract ServiceProviderRegistry is
1717
EIP712Upgradeable,
1818
ServiceProviderRegistryStorage
1919
{
20+
/// @notice Provider information for API returns
21+
struct ServiceProviderInfoView {
22+
uint256 providerId; // Provider ID
23+
ServiceProviderInfo info; // Nested provider information
24+
}
25+
2026
/// @notice Version of the contract implementation
2127
string public constant VERSION = "0.0.1";
2228

@@ -161,8 +167,7 @@ contract ServiceProviderRegistry is
161167
payee: payee,
162168
name: name,
163169
description: description,
164-
isActive: true,
165-
providerId: providerId
170+
isActive: true
166171
});
167172

168173
// Update address mapping
@@ -473,9 +478,10 @@ contract ServiceProviderRegistry is
473478
external
474479
view
475480
providerExists(providerId)
476-
returns (ServiceProviderInfo memory info)
481+
returns (ServiceProviderInfoView memory info)
477482
{
478-
return providers[providerId];
483+
ServiceProviderInfo storage provider = providers[providerId];
484+
return ServiceProviderInfoView({providerId: providerId, info: provider});
479485
}
480486

481487
/// @notice Get product data for a specific product type
@@ -548,9 +554,10 @@ contract ServiceProviderRegistry is
548554
for (uint256 i = 1; i <= numProviders && resultIndex < limit; i++) {
549555
if (providerProducts[i][productType].productData.length > 0) {
550556
if (currentIndex >= offset && currentIndex < offset + limit) {
557+
ServiceProviderInfo storage provider = providers[i];
551558
result.providers[resultIndex] = ProviderWithProduct({
552559
providerId: i,
553-
providerInfo: providers[i],
560+
providerInfo: provider,
554561
product: providerProducts[i][productType]
555562
});
556563
resultIndex++;
@@ -597,9 +604,10 @@ contract ServiceProviderRegistry is
597604
&& providerProducts[i][productType].productData.length > 0
598605
) {
599606
if (currentIndex >= offset && currentIndex < offset + limit) {
607+
ServiceProviderInfo storage provider = providers[i];
600608
result.providers[resultIndex] = ProviderWithProduct({
601609
providerId: i,
602-
providerInfo: providers[i],
610+
providerInfo: provider,
603611
product: providerProducts[i][productType]
604612
});
605613
resultIndex++;
@@ -625,9 +633,27 @@ contract ServiceProviderRegistry is
625633
/// @notice Get provider info by address
626634
/// @param providerAddress The address of the service provider
627635
/// @return info The provider information (empty struct if not registered)
628-
function getProviderByAddress(address providerAddress) external view returns (ServiceProviderInfo memory info) {
636+
function getProviderByAddress(address providerAddress)
637+
external
638+
view
639+
returns (ServiceProviderInfoView memory info)
640+
{
629641
uint256 providerId = addressToProviderId[providerAddress];
630-
return providers[providerId];
642+
if (providerId == 0) {
643+
return ServiceProviderInfoView({
644+
providerId: 0,
645+
info: ServiceProviderInfo({
646+
serviceProvider: address(0),
647+
payee: address(0),
648+
name: "",
649+
description: "",
650+
isActive: false
651+
})
652+
});
653+
}
654+
655+
ServiceProviderInfo storage provider = providers[providerId];
656+
return ServiceProviderInfoView({providerId: providerId, info: provider});
631657
}
632658

633659
/// @notice Get provider ID by address

‎service_contracts/src/ServiceProviderRegistryStorage.sol‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ contract ServiceProviderRegistryStorage {
2323
string name; // Optional provider name (max 128 chars)
2424
string description; //Service description, ToC, contract info, website..
2525
bool isActive;
26-
uint256 providerId; // Unique identifier for the provider
2726
}
2827

2928
/// @notice Product offering of the Service Provider

‎service_contracts/test/ServiceProviderRegistry.t.sol‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,11 @@ contract ServiceProviderRegistryTest is Test {
204204
);
205205

206206
// Verify provider info
207-
ServiceProviderRegistryStorage.ServiceProviderInfo memory info = registry.getProvider(providerId);
207+
ServiceProviderRegistry.ServiceProviderInfoView memory info = registry.getProvider(providerId);
208208
assertEq(info.providerId, providerId, "Provider ID should match");
209-
assertEq(info.serviceProvider, user1, "Service provider should be user1");
210-
assertEq(info.payee, user2, "Payee should be user2");
211-
assertTrue(info.isActive, "Provider should be active");
209+
assertEq(info.info.serviceProvider, user1, "Service provider should be user1");
210+
assertEq(info.info.payee, user2, "Payee should be user2");
211+
assertTrue(info.info.isActive, "Provider should be active");
212212
}
213213

214214
function testCannotRegisterWithZeroBeneficiary() public {
@@ -280,10 +280,10 @@ contract ServiceProviderRegistryTest is Test {
280280
);
281281

282282
// Now get provider should work
283-
ServiceProviderRegistryStorage.ServiceProviderInfo memory info = registry.getProvider(1);
283+
ServiceProviderRegistry.ServiceProviderInfoView memory info = registry.getProvider(1);
284284
assertEq(info.providerId, 1, "Provider ID should be 1");
285-
assertEq(info.serviceProvider, user1, "Service provider should be user1");
286-
assertEq(info.payee, user1, "Payee should be user1");
285+
assertEq(info.info.serviceProvider, user1, "Service provider should be user1");
286+
assertEq(info.info.payee, user1, "Payee should be user1");
287287
}
288288

289289
// Note: We can't test non-PDP product types since Solidity doesn't allow

0 commit comments

Comments
 (0)