24/Service registry by tahpot · Pull Request #28 · verida/blockchain-contracts

tahpot · 2022-05-15T07:21:50Z

Closes #24 .

Initial MVP of service registry.

Remaining: node-test/getlogs_*.js

BytesLib.sol Removed unnecessary functions VeridaDIDRegistry.sol Update function modifiers from 'public' to 'external'

- Add comments in the body of functions - Add discoverService() function

VDA-Service-Registry/contracts/ServiceRegistry.sol

tahpot · 2022-05-15T07:34:40Z

VDA-Service-Registry/contracts/ServiceRegistry.sol

+        bool isRegistered = registeredIds[identity].contains(serviceId);
+        require(!isRegistered, "Service has already registered!");
+        registeredIds[identity].add(serviceId);
+        AccountInfo storage _account = accountInfos[identity];


First check the account exists?

VDA-Service-Registry/contracts/ServiceRegistry.sol

VDA-Service-Registry/test/index.ts

VDA-Service-Registry/contracts/ServiceRegistry.sol

VDA-Service-Registry/.env.example

pranavburnwal · 2022-05-23T13:37:19Z

@BlockchainCrazy95 The comments are not quite in good order, can you please format function comments in this way.

/**
 * @dev Returns x raised to the n-th power.
 *
 * @param {number} x The number to raise.
 * @param {number} n The power, must be a natural number.
 * @return {number} x raised to the n-th power.
 */
function pow(x, n) {
  ...
}

Refer to this if you need some more understanding: https://javascript.info/comments
Ref: https://en.wikipedia.org/wiki/JSDoc

pranavburnwal · 2022-05-23T13:42:48Z

VDA-Service-Registry/scripts/deploy.ts

+
+  const tokenFactory = await ethers.getContractFactory("MockVDA");
+  const tokenContract = await tokenFactory.deploy();
+  await tokenContract.deployed();


Please add a condition that this doesn't deploy the same(VDA) in mainnet (ETH or MATIC)

Also, add the part that we can feed in the 'actual' VDA token address for mainnet launches.

I'm not sure what you mean. Can you kindly explain more about it?
It's a sample token for test. I won't deploy the Verida Token.

Testnet and mainnet deployment.

VDA-Service-Registry/test/index.ts

VDA-Service-Registry/contracts/ServiceRegistry.sol

pranavburnwal · 2022-05-24T04:09:07Z

Have one test case failing @ITStar10

BlockchainCrazy95 · 2022-05-24T13:23:06Z

Slither

Run slither and it showed below issues.

There were two high level severity issues and fixed.

ServiceRegistry._addCredit(address,uint256,address) ignores return value by vdaToken.transferFrom(sender,address(this), numCredit)
ServiceRegistry._removeCredit(address,uint256,address) ignores return value by vdaToken.transfer(sender,numCredit)

BlockchainCrazy95 · 2022-05-24T15:08:17Z

Mythril

No issues were detected.

Update public methods to external

Added comment in NatSpec Updated configuration : using .env

BlockchainCrazy95 · 2022-05-27T11:16:11Z

Have one test case failing @ITStar10

Please kindly explain about how the process of updating service will work. In case of decreasing the maxAccounts.

Remaining: node-test/getlogs_*.js

BytesLib.sol Removed unnecessary functions VeridaDIDRegistry.sol Update function modifiers from 'public' to 'external'

Added comment in NatSpec Updated configuration : using .env

…n-contracts into feature/10-did-registry

- Add comments in the body of functions - Add discoverService() function

ITStar10 · 2022-06-09T07:12:40Z

VDA-Service-Registry/contracts/ServiceRegistry.sol

+    struct AccountInfo {
+        address identity;
+        uint256 creditAmount;
+    }


No need AccountInfo.

ITStar10 · 2022-06-09T07:13:26Z

VDA-Service-Registry/contracts/ServiceRegistry.sol

+    /**
+     * did => AccountInfo
+     */
+    mapping (address => AccountInfo) accountInfos;


No need to use AccountInfo struct.

ITStar10 · 2022-06-09T07:14:36Z

VDA-Service-Registry/contracts/ServiceRegistry.sol

+        uint256 maxAccounts,
+        uint256 pricePerAccount,
+        bytes calldata signature
+    ) public onlyVerifiedSignature(identity, signature) {


Need to check out status of service.

ITStar10 · 2022-06-09T07:15:06Z

VDA-Service-Registry/contracts/ServiceRegistry.sol

+        address identity,
+        bytes32 serviceId,
+        bytes calldata signature
+    ) external onlyVerifiedSignature(identity, signature) {


Need to check service status.

VDA-Service-Registry/contracts/ServiceRegistry.sol

ITStar10

Need updates for credit. Or it can be resolved while implementing treasury.
Need service status check in functions

VDA-Service-Registry/contracts/ServiceRegistry.sol

ITStar10

Consolidate unnecessary functions.
Remove unnecessary checks on token transfer.

Remaining: CheckServiceAvailability for user User disconnect problem And other stuffs

…chain-contracts into feature/24-service-registry

ITStar10 and others added 4 commits May 9, 2022 14:57

Updated for gasles transaction server

d1bede9

Remaining: node-test/getlogs_*.js

Minor updte for Package.json

7baf6ee

Updates by Slither check

eddec24

BytesLib.sol Removed unnecessary functions VeridaDIDRegistry.sol Update function modifiers from 'public' to 'external'

Initial commit

ec32b3a

tahpot added the enhancement New feature or request label May 15, 2022

tahpot requested a review from pranavburnwal May 15, 2022 07:21

tahpot assigned BlockchainCrazy95 May 15, 2022

tahpot marked this pull request as draft May 15, 2022 07:31

tahpot requested a review from ITStar10 May 15, 2022 07:31

BlockchainCrazy95 marked this pull request as ready for review May 18, 2022 23:22

Comment Added

326e631

- Add comments in the body of functions - Add discoverService() function

BlockchainCrazy95 force-pushed the feature/24-service-registry branch from 6830a7a to 326e631 Compare May 19, 2022 08:38

tahpot commented May 19, 2022

View reviewed changes

Minor updates

dc4b0cd

pranavburnwal reviewed May 23, 2022

View reviewed changes

VDA-Service-Registry/.env.example Show resolved Hide resolved

pranavburnwal reviewed May 23, 2022

View reviewed changes

VDA-Service-Registry/test/index.ts Show resolved Hide resolved

pranavburnwal reviewed May 23, 2022

View reviewed changes

VDA-Service-Registry/test/index.ts Show resolved Hide resolved

pranavburnwal reviewed May 23, 2022

View reviewed changes

VDA-Service-Registry/contracts/ServiceRegistry.sol Show resolved Hide resolved

pranavburnwal reviewed May 23, 2022

View reviewed changes

VDA-Service-Registry/contracts/ServiceRegistry.sol Outdated Show resolved Hide resolved

BlockchainCrazy95 and others added 4 commits May 25, 2022 10:29

Update

58381f8

Updated visibility of functions

06ee410

Update public methods to external

Updates on Review

2899065

Added comment in NatSpec Updated configuration : using .env

Merge pull request #30 from verida/Alex/VeridaToken-Update

1a6b9ff

Updated for gasles transaction server

ab985d5

Remaining: node-test/getlogs_*.js

ITStar10 and others added 6 commits May 27, 2022 20:44

Minor updte for Package.json

a3b0896

Updates by Slither check

edff455

BytesLib.sol Removed unnecessary functions VeridaDIDRegistry.sol Update function modifiers from 'public' to 'external'

Updates on Review

318dee4

Added comment in NatSpec Updated configuration : using .env

Minor updates for hardhatconfig.ts

d660e46

Merge branch 'feature/10-did-registry' of github.com:verida/blockchai…

bcfea50

…n-contracts into feature/10-did-registry

Merge pull request #27 from verida/feature/10-did-registry

05144e8

nick-verida unassigned BlockchainCrazy95 May 31, 2022

pranavburnwal assigned ITStar10 Jun 1, 2022

BlockchainCrazy95 added 4 commits June 9, 2022 11:33

Initial commit

72b0894

Comment Added

bc4251a

- Add comments in the body of functions - Add discoverService() function

Minor updates

c781673

Update

1bf0b14

ITStar10 reviewed Jun 9, 2022

View reviewed changes

VDA-Service-Registry/contracts/ServiceRegistry.sol Outdated Show resolved Hide resolved

ITStar10 reviewed Jun 9, 2022

View reviewed changes

VDA-Service-Registry/contracts/ServiceRegistry.sol Outdated Show resolved Hide resolved

ITStar10 reviewed Jun 9, 2022

View reviewed changes

VDA-Service-Registry/contracts/ServiceRegistry.sol Outdated Show resolved Hide resolved

ITStar10 reviewed Jun 9, 2022

View reviewed changes

VDA-Service-Registry/contracts/ServiceRegistry.sol Outdated Show resolved Hide resolved

ITStar10 reviewed Jun 9, 2022

View reviewed changes

VDA-Service-Registry/contracts/ServiceRegistry.sol Outdated Show resolved Hide resolved

ITStar10 reviewed Jun 9, 2022

View reviewed changes

VDA-Service-Registry/contracts/ServiceRegistry.sol Outdated Show resolved Hide resolved

ITStar10 reviewed Jun 9, 2022

View reviewed changes

tahpot marked this pull request as draft June 13, 2022 01:53

ITStar10 added 2 commits June 21, 2022 16:30

Update in progress

1a72936

Remaining: CheckServiceAvailability for user User disconnect problem And other stuffs

Merge branch 'feature/24-service-registry' of github.com:verida/block…

11e0fa3

…chain-contracts into feature/24-service-registry

Conversation

tahpot commented May 15, 2022

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pranavburnwal commented May 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pranavburnwal commented May 24, 2022

Uh oh!

BlockchainCrazy95 commented May 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Slither

Uh oh!

BlockchainCrazy95 commented May 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Mythril

Uh oh!

BlockchainCrazy95 commented May 27, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ITStar10 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ITStar10 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pranavburnwal commented May 23, 2022 •

edited

Loading

BlockchainCrazy95 commented May 24, 2022 •

edited

Loading

BlockchainCrazy95 commented May 24, 2022 •

edited

Loading