kesar / ethereum-php

Ethereum Client in PHP
103 stars 50 forks source link

No error check leads to chrash #8

Open notreadyyet opened 6 years ago

notreadyyet commented 6 years ago

May I ask whether you intend to change classes derived from AbstractMethods to check for errors returned by ClientInterface? I see that three days ago Personal::unlockAccount was changed to check for errors, but IMHO that change didn’t go far enough. I think that to be useful in the real-world applications all functions should call getRpcErrorCode before calling getRpcResult. Also I think that error code and error message should be stored by the AbstractMethods alongside the $client variable to be retrieved by EthereumClient.

notreadyyet commented 6 years ago

Please find in the attachment the changes I've made locally. Methods.zip The getTransactionByHash method that used to crash because of this error: {"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"authentication needed: password or unlock"}} now looks like this:

public function getTransactionByHash(TransactionHash $hash): ?TransactionInfo
{
    $response = $this->client->send(
        $this->client->request(1, 'eth_getTransactionByHash', [$hash->toString()])
    );
    if (!$this->fnHasRes($response)) {
        return null ;
    }

    return new TransactionInfo($this->getRpcResult()) ;
}
notreadyyet commented 6 years ago

Oups wrong method. It was sendTransaction that crashed, but the change is the same: public function sendTransaction(Transaction $transaction): ?TransactionHash { $response = $this->client->send( $this->client->request(1, 'eth_sendTransaction', [$transaction->toArray()]) ); if (!$this->fnHasRes($response)) { return null ; }

    return new TransactionHash($this->getRpcResult());

}
notreadyyet commented 6 years ago

Now the client instead of crashing can retrieve error information and do something about it like so:

    $transaction = new \EthereumPHP\Types\Transaction(
            $myAddress,
            $otherAddress,
            null,
            null,
            null,
            (new \EthereumPHP\Types\Ether(5))->toWei()->amount()
            );
    $t1 = $client->eth()->sendTransaction($transaction) ;      // this can return null
    $oErr = $client->eth()->fnGetRes() ;                // this allows us to find out why $t1 is null
    if (isset($t1)) {
        $t1 = $t1->toString();
    }