Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(pip): implement PIP-23 #1397

Merged
merged 21 commits into from
Jul 12, 2024
Merged

Conversation

ZigBalthazar
Copy link
Contributor

@ZigBalthazar ZigBalthazar commented Jul 4, 2024

@ZigBalthazar ZigBalthazar requested review from b00f and kehiy July 4, 2024 20:14
kehiy
kehiy previously requested changes Jul 4, 2024
Copy link
Contributor

@kehiy kehiy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run make proto and make sure it finishes properly.

@kehiy kehiy changed the title feat: implementation for pip-23 feat(pip): implementing PIP-23 Jul 4, 2024
@kehiy kehiy added the Wallet label Jul 4, 2024
@ZigBalthazar ZigBalthazar requested a review from kehiy July 4, 2024 20:50
Copy link
Contributor

@Ja7ad Ja7ad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write utils service (utils.proto) for sign and verify message outside of wallet.

    - selector: pactus.Utils.SignMessage
      get: "/pactus/utils/sign_message"

    - selector: pactus.Utils.VerifyMessage
      get: "/pactus/utils/verify_message"

https://pips.pactus.org/PIPs/pip-23#specification

util/crypto/crypto.go Outdated Show resolved Hide resolved
util/crypto/crypto_test.go Outdated Show resolved Hide resolved
util/crypto/crypto_test.go Outdated Show resolved Hide resolved
@ZigBalthazar
Copy link
Contributor Author

Hi @Ja7ad ,
in my opinion we should not put the sign message in grpc cause of the sign message need private Key and if we transfer the private key on network could make some security vulnerability

@Ja7ad
Copy link
Contributor

Ja7ad commented Jul 5, 2024

Hi @Ja7ad ,
in my opinion we should not put the sign message in grpc cause of the sign message need private Key and if we transfer the private key on network could make some security vulnerability

You should do it, it don't have vulnerability.

util/crypto/crypto.go Outdated Show resolved Hide resolved
wallet/wallet_test.go Outdated Show resolved Hide resolved
www/grpc/proto/util.proto Outdated Show resolved Hide resolved
www/grpc/buf/grpc-gateway.config.yaml Outdated Show resolved Hide resolved
www/grpc/buf/grpc-gateway.config.yaml Outdated Show resolved Hide resolved
wallet/wallet_test.go Outdated Show resolved Hide resolved
@ZigBalthazar ZigBalthazar requested review from Ja7ad and b00f July 11, 2024 16:50
wallet/manager.go Outdated Show resolved Hide resolved
wallet/wallet.go Outdated Show resolved Hide resolved
wallet/wallet_test.go Outdated Show resolved Hide resolved
wallet/wallet_test.go Outdated Show resolved Hide resolved
www/grpc/buf/grpc-gateway.config.yaml Outdated Show resolved Hide resolved
www/grpc/proto/utils.proto Outdated Show resolved Hide resolved
www/grpc/proto/utils.proto Outdated Show resolved Hide resolved
www/grpc/proto/utils.proto Outdated Show resolved Hide resolved
www/grpc/proto/utils.proto Outdated Show resolved Hide resolved
www/grpc/proto/utils.proto Outdated Show resolved Hide resolved
www/grpc/proto/utils.proto Outdated Show resolved Hide resolved
www/grpc/proto/utils.proto Outdated Show resolved Hide resolved
www/grpc/proto/utils.proto Outdated Show resolved Hide resolved
www/grpc/proto/utils.proto Outdated Show resolved Hide resolved
www/grpc/proto/utils.proto Show resolved Hide resolved
www/grpc/buf/grpc-gateway.config.yaml Outdated Show resolved Hide resolved
www/grpc/buf/grpc-gateway.config.yaml Outdated Show resolved Hide resolved
@Ja7ad
Copy link
Contributor

Ja7ad commented Jul 11, 2024

@ZigBalthazar please register utils service in json-rpc server.

https://github.com/pactus-project/pactus/blob/main/www%2Fjsonrpc%2Fserver.go#L57-L60

Also for grpc gateway too:

https://github.com/pactus-project/pactus/blob/main/www%2Fgrpc%2Fgateway.go#L55-L67

@ZigBalthazar ZigBalthazar requested a review from Ja7ad July 11, 2024 18:58
@Ja7ad
Copy link
Contributor

Ja7ad commented Jul 11, 2024

@ZigBalthazar Please generate proto for new changes.

@b00f b00f changed the title feat(pip): implementing PIP-23 feat(pip): implement PIP-23 Jul 12, 2024
@b00f
Copy link
Collaborator

b00f commented Jul 12, 2024

@ZigBalthazar Thanks

@Ja7ad
Copy link
Contributor

Ja7ad commented Jul 12, 2024

@ZigBalthazar Please regenerate proto.

@Ja7ad Ja7ad dismissed kehiy’s stale review July 12, 2024 10:05

Please check.

@Ja7ad Ja7ad merged commit 50b5eb5 into pactus-project:main Jul 12, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PIP-23: Sign and verify message by public and private key account
4 participants