verze-app / solana-php-sdk

Simple PHP SDK for Solana JSON RPC endpoints
MIT License
88 stars 45 forks source link

isOnCurve not right #31

Closed pantaovay closed 2 years ago

pantaovay commented 2 years ago

isOnCurve not right

Base 58 Publickey: q5xfHVuAsGmKRTmJNshERHHkVnHKxRX3cKzz5cYnFRn

It show return true for isOnCurve function. But false returned

neverything commented 2 years ago

@pantaovay not sure I'm getting the problem. Does it return trueor false for isOnCurve?

Sidenote, I had an issue with the getProgramAddress and solved it by checking multiple nonce values, see https://github.com/tighten/solana-php-sdk/issues/12#issue-1027315555 for details.

pantaovay commented 2 years ago

@neverything

For address q5xfHVuAsGmKRTmJNshERHHkVnHKxRX3cKzz5cYnFRn, it is on curve, the function should return true. Buf it returns false.

I checked #12 , the reason that the function generates false PDA is isOnCurse issue. If isOnCurve function works right, it has no issue.

I use another way to avoid this because the isOnCurve function in the other js lib is very difficult.

    public static function isOnCurve(PublicKey $publicKey): bool
    {
        try {
            $response = Di::getHttpClient()->sendGetRequest(sprintf(
                'http://127.0.0.1:3000/publickey/%s/is_on_curve',
                $publicKey->toBase58()
            ));

            $responseBody = json_decode($response->getBody()->getContents(), true);
            if (!is_array($responseBody) || !isset($responseBody['is_on_curve'])) {
                throw new \Exception($responseBody['error_code']);
            }

            return (bool) $responseBody['is_on_curve'];
        } catch (\Exception $exception) {
            Di::getLogger()->error('Check is on curve error: ' . $exception->getMessage());

            return false;
        }
    }
gabrielkoerich commented 2 years ago

I found out here that sodium_crypto_sign_ed25519_pk_to_curve25519($binaryString) is almost always returning a SodiumException with "conversion failed" message.

Using \ParagonIE_Sodium_Compat::crypto_sign_ed25519_pk_to_curve25519($binaryString) I was able to get the correct address

neverything commented 2 years ago

@gabrielkoerich good catch, this does indeed seem like a bug. Feel free to write a pull request for it or I will eventually get to it in the future as it's bothering me a little 😄

neverything commented 2 years ago

This is fixed in https://github.com/verze-app/solana-php-sdk/commit/7b1e6ae31d765917eea6812ca7262f7a80d9a8fd thanks to @gabrielkoerich

Not creating a new release yet, see https://github.com/verze-app/solana-php-sdk/discussions/36