monero-integrations / monerophp

Monero PHP library + JsonRPC Client
MIT License
119 stars 76 forks source link

Fix verifying checksum for integrated addresses #150

Closed itismadness closed 10 months ago

itismadness commented 10 months ago

Fix a bug in the Cryptonote::verify_checksum function where it would fail on integrated addresses where these addresses are 154 characters when decoded (106 encoded), where the checksum is the last 8 characters, similar to regular addresses which decode to 138 characters.

By using a negative offset for substr, this will grab the last 8 characters, regardless of length of $decoded, which helps future proof this in the case a new address type is ever introduced.

As a testcase, the monerodocs on integrated addresses provides a sample address:

4LL9oSLmtpccfufTMvppY6JwXNouMBzSkbLYfpAV5Usx3skxNgYeYTRj5UzqtReoS44qo9mtmXCqY45DJ852K5Jv2bYXZKKQePHES9khPK

where on master:

php > $c = new MoneroIntegrations\MoneroPhp\Cryptonote();
php > var_dump($c->verify_checksum('4LL9oSLmtpccfufTMvppY6JwXNouMBzSkbLYfpAV5Usx3skxNgYeYTRj5UzqtReoS44qo9mtmXCqY45DJ852K5Jv2bYXZKKQePHES9khPK'));
bool(false)

and with this patch:

php > $c = new MoneroIntegrations\MoneroPhp\Cryptonote();
php > $c->verify_checksum('4LL9oSLmtpccfufTMvppY6JwXNouMBzSkbLYfpAV5Usx3skxNgYeYTRj5UzqtReoS44qo9mtmXCqY45DJ852K5Jv2bYXZKKQePHES9khPK');
php > var_dump($c->verify_checksum('4LL9oSLmtpccfufTMvppY6JwXNouMBzSkbLYfpAV5Usx3skxNgYeYTRj5UzqtReoS44qo9mtmXCqY45DJ852K5Jv2bYXZKKQePHES9khPK'));
bool(true)

The regular address provided in #135 returns true for both master and this branch.

serhack commented 10 months ago

Thank you, feel free to contribute since this repo needs more love