ethereum / ethereumj

DEPRECATED! Java implementation of the Ethereum yellowpaper. For JSON-RPC and other client features check Ethereum Harmony
GNU Lesser General Public License v3.0
2.18k stars 1.09k forks source link

In ethereumj, why the signature use loop to get v? #1261

Closed woshidadashi closed 5 years ago

woshidadashi commented 5 years ago

In the java version of Ethereum, I see the signature code as follows:

public ECDSASignature sign(byte[] messageHash) {
    ECDSASignature sig = doSign(messageHash);
    // Now we have to work backwards to figure out the recId needed to recover the signature.
    int recId = -1;
    byte[] thisKey = this.pub.getEncoded(/* compressed */ false);
    for (int i = 0; i < 4; i++) {
        byte[] k = ECKey.recoverPubBytesFromSignature(i, sig, messageHash);
        if (k != null && Arrays.equals(k, thisKey)) {
            recId = i;
            break;
        }
    }
    if (recId == -1)
        throw new RuntimeException("Could not construct a recoverable key. This should never happen.");
    sig.v = (byte) (recId + 27);
    return sig;
}

Why this code use loop (for (int i = 0; i < 4; i++)) to get v (aka recId)? It can change the signature code and get v from the signing process. Can you please tell me? Thank you.

netwalker2000 commented 5 years ago

I also made a great effort to understand this part too. There can be up to 4 different points with a given "X coordinate modulo n". (2 because each X coordinate has two possible Y coordinates, and 2 because r+n may still be a valid X coordinate). That number between 0 and 3 we call the recovery id, or recid. Anyone who has a better idea tells me too..

mkalinin commented 5 years ago

There are two possible recId values for SECP256K1: 0 and 1.

@woshidadashi what you're suggesting requires obtaining y coordinate corresponding to r component which is in its turn calculated with usage of SpongyCastle library. And iteration here is the only way of getting this bit for recId without modifying an underlying crypto lib code.

woshidadashi commented 5 years ago

I tried, this is very easy to modify BC lib.

mkalinin commented 5 years ago

Maybe you wanna make a PR and then we can see how intrusive the change is?

woshidadashi commented 5 years ago

image Change the ECDSASigner.java like this, this can make a 3 times speed up following my test.

mkalinin commented 5 years ago

Thanks for pointing that out. But changing a crypto provider code is quite a risky step. Since speeding up signature generation does not affect processing speed I'd rather keep it as it is now.