Skip to content

The SDK is finished#1

Open
klausbyskov wants to merge 10 commits intomasterfrom
Dev
Open

The SDK is finished#1
klausbyskov wants to merge 10 commits intomasterfrom
Dev

Conversation

@klausbyskov
Copy link
Collaborator

Please merge into master.

/// <param name="sendBlock">Send block</param>
/// <param name="privateKey">Private key, if not set, will return block without signature and work</param>
/// <returns>Receive block</returns>
public async Task<QlcResponse<Block>> GenerateReceiveBlockAsync(Block sendBlock, string privateKey = null)

Choose a reason for hiding this comment

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

Great Job!
An issue for GenerateReceiveBlockAsync is that it is not safe to pass the user's private key to a remote server, so it's needed a local method to complete the signature of the block when no private key provided in this call.
It is same for GenerateSendBlockAsync and GenerateChangeBlockAsync.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, no problem. I will make such a method. Do you have any documentation on how exactly the signature should be computed?

Choose a reason for hiding this comment

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

Ok, no problem. I will make such a method. Do you have any documentation on how exactly the signature should be computed?

The signature is using a modified Ed25519, the modification is the hash function is using Blake2b instead of Sha256 in default Ed25519.

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

@klausbyskov klausbyskov Jul 24, 2019

Choose a reason for hiding this comment

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

Thank you. And the data to generate the signature from? Is it just the block serialized as Json? Or is there something else I need to know?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, ok, I will take a look at the go code, and do it the same way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So I've looked into ed25519 and blake2b, and there are basically no full implementations in .net that have the features that you use in your Sign method.
I could write everything from scratch by porting the go code, but I feel that this is a task that is way out of scope for this SDK, and probably should be coded by someone with deep insight into cryptography (ie. not me).

The implementation of your algorithm would probably belong in it's own library, and shouldn't be part of the SDK (only referenced by it), as it could just as well stand alone, and would need to be thoroughly tested by someone who understands every single thing it does.

Choose a reason for hiding this comment

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

So I've looked into ed25519 and blake2b, and there are basically no full implementations in .net that have the features that you use in your Sign method.
I could write everything from scratch by porting the go code, but I feel that this is a task that is way out of scope for this SDK, and probably should be coded by someone with deep insight into cryptography (ie. not me).

The implementation of your algorithm would probably belong in it's own library, and shouldn't be part of the SDK (only referenced by it), as it could just as well stand alone, and would need to be thoroughly tested by someone who understands every single thing it does.

It shall be very convenient have this local sign method, especially for developer that developing a light wallet using .net SDK.

I searched some open source of Blake2b and Ed25519 for reference:
https://github.com/BLAKE2/BLAKE2/blob/master/csharp/Blake2Sharp/Blake2BCore.cs
https://github.com/floodyberry/ed25519-donna/blob/master/ed25519.c

Or, we can put it as the next stage task.
Thanks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree that it would be extremely convenient to have.

The ed25519 class you have linked is written in C, not C#.
The Blake2b implementation you have linked only has a Hash method. Your own implementation uses methods called sum and write that I don't know what do exactly, but it seems unlikely (to me) that calling Hash would do the same.

For a future implementation of local versions of the Sign/Verify methods, I feel that it would be extremely helpful if you could provide some extensive test data for verification purposes.
I'm thinking something like this:

{
    "sign": [
        {
            "privateKey": "asaadasdas...",
            "message": "asfsfhfhfg...",
            "signature": "dgdfdhfghgf..."
        },
        ...
    ],
    "verify": [
        {
            "publicKey": "adsasdasda...",
            "message": "sdfgsdgdfg...",
            "signature": "dgdfd45fsdf..."
        }
        ...
    ]
}

Your own test here https://github.com/qlcchain/qlc-go-sdk/blob/34e9c2ae052b710134b9a6ad033a81a10bcc0555/pkg/ed25519/ed25519_test.go#L39 gives very little help for implementations in another language, because someone could write a similar test that passed, but without generating the same signature as the golang code.

Choose a reason for hiding this comment

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

I agree that it would be extremely convenient to have.

The ed25519 class you have linked is written in C, not C#.
The Blake2b implementation you have linked only has a Hash method. Your own implementation uses methods called sum and write that I don't know what do exactly, but it seems unlikely (to me) that calling Hash would do the same.

For a future implementation of local versions of the Sign/Verify methods, I feel that it would be extremely helpful if you could provide some extensive test data for verification purposes.
I'm thinking something like this:

{
    "sign": [
        {
            "privateKey": "asaadasdas...",
            "message": "asfsfhfhfg...",
            "signature": "dgdfdhfghgf..."
        },
        ...
    ],
    "verify": [
        {
            "publicKey": "adsasdasda...",
            "message": "sdfgsdgdfg...",
            "signature": "dgdfd45fsdf..."
        }
        ...
    ]
}

Your own test here https://github.com/qlcchain/qlc-go-sdk/blob/34e9c2ae052b710134b9a6ad033a81a10bcc0555/pkg/ed25519/ed25519_test.go#L39 gives very little help for implementations in another language, because someone could write a similar test that passed, but without generating the same signature as the golang code.

OK, Will provide the test data later

Copy link

@lc3091 lc3091 left a comment

Choose a reason for hiding this comment

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

1: Maybe you can add some testing case for every interface, that way i can verify them very easily.
2: Can you explain how to use this SDK in detail in the README?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants