Roadmap discussion about monerophp HOT 37 OPEN

Hi all,

Just ran into this discussion a couple days ago, coincidentally I have also been working on a rpc library: https://github.com/refactor-ring/monero-rpc-php

It is pretty complete, it's php 8.1+ and completely strong typed, all the json-rpc methods have been implemented according to the docs.

All the (de)serialization of the models is covered by unit tests and the aim is to cover as much as possible with integration tests against a docker instance of the daemon and wallet rpc servers.

Currently there are already integration tests running automatically and the goal is to keep on adding tests.

I also think it would be good to split up the current monerophp project into separate projects/packages.

from monerophp.

I forked @BrianHenryIE's fork to dockerize the project. commit: https://github.com/lpoirothattermann/bh-php-monero-rpc/commit/177867ae779e836c36db134456b33f1036c06f66

A docker-compose.yml to orchestrated containers easily.
Two containers:

• One for development environment with PHP 8.1 and Composer
• Another one for to have a Monero node locally, and hit him for local unit-tests.

Unit-tests is a big topic, do we mock every calls based on parameters input to determine the node response ? Or we use a public node with real calls ?

PS: I also added a Makefile to make things easier

from monerophp.

Yes, that is currently on my list of items to replace the libraries. In #141, I've deleted SHA3.php and replaced it with kornrunner/keccak.

I tried out tuupola/base58, but the current base58.php does a few conversions and padding, which I skipped for now.
I did not look for an ed25519 library replacement, I will look into simplito/elliptic-php.

Most of my PHP experience is out of PHP 8, so I'm quite new to strict types, and I've been wondering how to strictly-type associative arrays. Your xmrchain.net client repository is very helpful.

from monerophp.

Run composer lint on the project and there are a bunch of errors and warnings brought up.

One that comes up a few times is that some Wallet functions allowed the first parameter to be an array which was somewhat equivalent to the list of parameters. Personally, I don't like that, since to build the array is actually more difficult that calling a function with the same parameters:

$result = callFunction( $param1, $param2 );
// vs.
$params = [ 'param1' => $param1, 'param2' => $param2 ];
$result = callFunction( $params );
// and I think this is the same as the first call:
$result = callFunction( list( $params ) );
// and now modern PHP allows named parameters, so the list is much less ambiguous (consider with multiple optional parameters)
$result = callFunction( param1: $param1, param2: $param2 );

One fun problem to work on is adding authentication to the daemon connection. I really haven't looked at it, just that I know daemons and wallets can require username&password so I made an empty function to remind myself to work on it.

As we discussed in another ticket, the PHPCS standard right now is just PSR-12, which is the common base for many better/stricter standards. At the very least, I'd like to see PHPCS throw warnings when the Interfaces don't have comments, but I don't know off-hand what a good PHPCS standard to use would be (I use WordPress Coding Standard in my day-to-day, which does do that but obviously isn't what we want). I think doctrine was the one someone mentioned. Maybe give it a spin and judge is it strict enough/just right/too strict!

Types are certainly a goal, but I think (as in the previous comment) we should be able to auto-generate a majority of that code.

To do that, sample, repeatable, daemon calls are the magic sauce. If you/one could take a look at the Wallet daemon commands that might return the same value each time, that would be very helpful for the type code-generation.

from monerophp.

My changes are here: https://github.com/recanman/bh-php-monero-rpc

recanman/patch1 fixes a warning with PHPUnit.
recanman/patch2 implements digest authentication

from monerophp.

It's on my TODO list, will do on weekend

from monerophp.

I've refactored almost all of the crypto code and added tests. I have also deprecated the current wallet/daemon RPC connectors and merged refring's code. Going to make a PR once I finish everything.

There were a lot of type errors with bcmath operations that made the code incompatible with PHP 8. I've preserved functionality except for a small breaking change in the Varint class (use a string instead of an array for pop_varint as all other operations use strings).

I'm thinking that two packages should be made:

• monero-integrations/monero-crypto
  • Has base58, varint, ed25519, and cryptonote. Essentially, all math/crypto operations that one needs to do for low-level Monero work.
• monero-integrations/monero-rpc
  • Primary composed of refring's library. For connecting/interacting with daemon/wallet RPC.

from monerophp.

Look at issues, those are what can be done to contribute.

@serhack can you close this?

from monerophp.

I started work on the RPC client classes to get them strongly typed and unit tested.

https://github.com/BrianHenryIE/bh-php-monero-daemon-rpc

I'll do more work on it as the mood takes me. Hopefully will have the "work in progress" notice removed by Christmas!

from monerophp.

Thank you for your help.

from monerophp.

I did some more work on that today. I got some neat tests running where the RPC response was verified against the CLI output from monerod.

I think the RPC clients should be a different package to this one. The math content of this package isn't needed when using the RPC functions. I notice the Python RPC client is actually part of the monero-project/monero repo.

How do you feel about using external libraries instead of base58.php, ed25519.php and SHA3.php?

A quick search finds these decent looking libraries:

• 0xbb/php-sha3
• tuupola/base58
• simplito/elliptic-php

That would pare this package down to a size much easier to focus on when improving PhpDoc etc.

I'm also almost finished writing a client for xmrchain.net and its implementations: bh-php-monero-explorer

As I said above, this is something I dip into every now and then and will probably take many months to complete. But you can also see I'm aiming for very high code quality.

My real goal is to start writing PRs against the WordPress/WooCommerce plugin.

from monerophp.

I did a bunch more work on Daemon and Wallet RPC classes yesterday.

But then, when I started to write Wallet tests monerod segfaulted and when I rebooted my laptop it wouldn’t turn back on, so now it’s now away with Apple for repair!

@serhack Would you please take a look at https://github.com/BrianHenryIE/bh-php-monero-rpc and consider moving the repo into the monero-integrations organization (and renaming, obviously)? I think it should be its own repo, separate from this one but required by it.

It’s not “complete” but I think it’s as good as or better than the existing code (naturally, since that was the starting point!).

There is currently a PHP 8 requirement due to the JSON RPC library I’m using. And to reiterate my thoughts on PHP 7 from the other thread, nobody in their right mind is starting a new project using PHP 7 these days, and if they really need it, there is a tool called Rector which can transpile your code down to older PHP versions.

And @recanman , thanks for the motivation!

from monerophp.

You're welcome. How your computer 'stopped working' after monerod segfaulted is quite odd, I'd like to hear more on that if it ends up working again.

I've taken a look at elliptic-php, and it is a nice library, but like in base58.php, there are many helper functions that are included with the library. I am not a cryptographer, so I think it might be better to leave ed25519.php where it is right now, and add PHPDoc strings (which I'm doing in #140).

from monerophp.

Weird to read about your computer broken after running monerod. I'm guessing it probably corrupted the entire filesystem (although a user-space program should never be able to corrupt the fs). Keep us updated about it.

Regarding the bh-php-monero-rpc, it's a great repository but I'm still having doubts about its utility. What advantages could you have by using bh-php-monero-rpc instead of monerophp? If there's a real advantage, I'd like to have that in the monerophp repo after I merge all the PR.

from monerophp.

@serhack Would you please take a look at https://github.com/BrianHenryIE/bh-php-monero-rpc and consider moving the repo into the monero-integrations organization (and renaming, obviously)? I think it should be its own repo, separate from this one but required by it.

Why not merge bh-php-monero-rpc into monerophp?

from monerophp.

I think it might be better to leave them separate for now, and merge some changes from the bh-php-monero-rpc repository into here selectively. I will get some time this week, and finish off #140. Then, I will abstract (according to PSR standards) #138 and get it ready to merge. The reason I'm focusing on documentation is because it is inconsistent, and not helpful. Being able to have intellisense in the IDE saves hours of time, so auto-generated documentation based on parameters will be very helpful.

The biggest things remaining after that are strong types and unit tests. Unit tests shouldn't be implemented before strong types.
This is something I do not have much experience with, so I will use bh-php-monero-rpc as a reference.
Unit tests are just a matter of tedious work.

from monerophp.

I didn't mean to suggest monerod had broken my computer, I thought the issue with the computer was causing monerod to crash. Turns out it's an issue with M1 Macs (with a solution!).

I'd like to have that in the monerophp repo

The point in Composer is to allow splitting projects into packages which can then be reused independently. Each project has a "dependency tree". E.g. here's the JsonRpc library I'm using:

$ composer show --tree --no-dev
psr/http-factory 1.0.2 Common interfaces for PSR-7 HTTP message factories
├──php >=7.0.0
└──psr/http-message ^1.0 || ^2.0
   └──php ^7.2 || ^8.0
psr/http-message 1.1 Common interface for HTTP messages
└──php ^7.2 || ^8.0
thecodingmachine/safe v2.5.0 PHP core functions that throw exceptions instead of returning FALSE on error
└──php ^8.0

You can see there that psr/http-message doesn't depending on anything (except php), but psr/http-factory depends on psr/http-message. I.e. I can use psr/http-message on its own, when that's all I need, but it is automatically pulled in anytime I say I need psr/http-factory.

In the case ofmonero-integrations/monerophp, I see that the daemon rpc clients are something that might be used independently of the math code, and vice-versa. I expect I will need the math code when using the http Monero Explorer but not the rpc code, so I'd go so far as to suggest another fork of this repo which would then contain only the math code. This parent repo can still pull in all the monero php libraries under the one umbrella, but they could each be used independently.

If that were the case, Guzzle could reasonably be used in this repo.

So a consumer running composer install monero-integrations/monerophp would get something like this (abbreviated):

$ composer show --tree --no-dev
monero-integrations/monerophp 1.0.2  Monero PHP library + JsonRPC Client 
├──php >=7.0.0
├──monero-integrations/monero-rpc 1.0 Monero JsonRPC Client 
│  └──php ^8.0
├──monero-integrations/monero-math 1.0 Monero PHP library
│  ├──php ^7.2 || ^8.0
│  └──kornrunner/keccak ^1.1 Pure PHP implementation of Keccak (SHA-3) 
├──BrianHenryIE/bh-php-monero-explorer 1.0 Onion Monero Blockchain Explorer Client
│  └──php ^7.4 || ^8.0
└──guzzlehttp/guzzle 7.8.0 Guzzle is a PHP HTTP client library
   ├──ext-json *
   ├──guzzlehttp/promises ^1.5.3 || ^2.0.1
   │  └──php ^7.2.5 || ^8.0 
   ├──guzzlehttp/psr7 ^1.9.1 || ^2.5.1
   │  ├──php ^7.2.5 || ^8.0
   │  ├──psr/http-factory ^1.0
   │  │  ├──php >=7.0.0
   │  │  └──psr/http-message ^1.0 || ^2.0
   │  │     └──php ^7.2 || ^8.0
   │  ├──psr/http-message ^1.1 || ^2.0
   │  │  └──php ^7.2 || ^8.0
   │  └──ralouphie/getallheaders ^3.0
   │     └──php >=5.6
   ├──php ^7.2.5 || ^8.0
   ├──psr/http-client ^1.0
   │  ├──php ^7.0 || ^8.0
   │  └──psr/http-message ^1.0 || ^2.0
   │     └──php ^7.2 || ^8.0
   └──symfony/deprecation-contracts ^2.2 || ^3.0
      └──php >=8.1

Again, I'm happy to move it under the monero-integrations namespace, but I am sure it should be its own package.

It's also much easier to approach smaller libraries, both as a consumer and a contributor. BrianHenryIE/bh-php-monero-rpc is already at about 35 classes and I expect that to at least double.

Smaller packages are easier to understand, naturally. This makes them easier to contribute to. The separation of concerns also makes them easier to contribute to – I've much less imposter syndrome contributing to an rpc wrapper than an cryptographic library. Smaller packages can be used independently to keep installed code low, so when something is wrong, I'm not searching through dead code to try understand where the problem is.

Earlier I misinterpreted the question "What advantages...?" as asking why not use the current untyped stdclass responses, so I made this video showing the intellisense autocomplete and inline documentation:

Really, there's no disadvantage to breaking this library into smaller packages, because someone installing this will still get all of them automatically.

from monerophp.

I'm agree that we should split current package into multiple parts, not because it's easier to manage, with good project structure code wouldn't be messy, but because other people could want to use for example monero-integrations/monero-math or monero-integrations/monero-rpc for their own projects.
But I don't get way you want to create a new package under monero-intergrations that will just be the overall the same this one.

We could maybe start to create dependencies packages like monero-integrations/monero-math (the exhaustive list of package should be defined first) and once the package look finished/"ready to be use" in monero-integrations/monerophp we require it in composer and start to implement it, step by step, dependency package by dependency package.

from monerophp.

After looking into it more closely, I think that it would be better to work on bh-php-monero-rpc, and then split into packages and merge here.

from monerophp.

I have some ideas to make development go faster... although I've been working on other projects (including contributing to the PhpJsonRpc dependency that bh-php-monero-rpc uses).

The bh-php-monero-rpc Contributing section isn't 100% correct – that was what I was working on when my laptop hit the bucket.

Since bh-php-monero-rpc is using the PSR packages, it should be possible to extend a Psr\Http\Client\ClientInterface so the response is written to disk each time. Then we're auto-generating the tests/_data json files.

I had been using jacobdekeizer.github.io/json-to-php-generator website/web-app to generate the PHP, but no doubt there is an actual PHP library which can similarly input JSON and output PHP, maybe needing Swagger as a mid-step.

I think some ChatGPT/LLM can be leveraged for writing the docblock comments, with someone with more experience/authority (obviously looking at you @serhack !) to review that each is correct.

The hard part then is setting up the tests against the Monero daemon. Obviously needs to not use mainnet, and AIUI testnet is for development of Monero itself, so stagenet seems to be the correct network to use. While I say this is the hard part, I think it is a particularly valuable part of the documentation.

One bad thing is, it seems the PhpJsonRpc dependency depends on PHP 8.1 now, which I didn't realise before. Since there's so much development work to do still, I'm not in a hurry to address that. I presume, but I'm not certain, there is a way to publish parallel versions of packages on Packagist for different PHP versions, i.e. what I said previously about Rector but done at the package level rather than the consumer level.

Sorry for the brain-dump! I pushed a couple of commits to bh-php-monero-rpc just now – only major thing was the PhpJsonRpc changes.

I think another reason to move brianhenryie/bh-php-monero-rpc to monero-integrations/php-monero-rpc (or whatever) is so contributing it looks like it's contributing to a community project and not just Brian Henry's project.

from monerophp.

Great, thank you. Other than testing for the currently-opened issues and making sure that those problems don't exist, is there anything else that should be done? I see in the repository that you said that one class just returns stdClass, so types are a good point to start from.

from monerophp.

Thanks for the summary. I am still brushing up on modern PHP.
I'll look into adding authentication. It is HTTP Digest Access authentication. (rfc 7616)

from monerophp.

So I looked into it, and I haven't found any client RFC 7616 implementations that don't depend on an external library. Implementation of digest auth isn't too bad and is pretty simple.

from monerophp.

That is fantastic work. At the very least, that should be added to the README in this repo today!

I've written some tests which are verified against the result of running monerod CLI commands via shell_exec(). That's fine locally, but not really a runner for CI systems. Have you come across any other projects which have that part figured out? I'm imagining a Docker image with a short blockchain already downloaded, which is set not to connect to other peers, so should be ready to test against immediately.

from monerophp.

This work is phenomenal. Your code is great and is very organized. The next crucial part is getting this all together and getting it ready to merge here.
I believe that having a very short piece of the Monero blockchain and running monerod with peering off will be good for testing.

At this point, all is left is the list you have on your repository, and making sure the issues here do not occur with your code. Separating the packages is also on the list. I see that you don't have any cryptography in your library, so the next thing for me is beginning work to separate the packages (and also implementing strict types).

Here's an overview of what's left:

• Finish types on library
• Finish tests on library
• Other items on your list (including error handling)
• Making sure all issues from this repository are covered
• Cleaning up ed25519.php and other important code in that category
• Separating packages

Thank you very much for your contribution.

from monerophp.

Great work guys! The minimum I should do is giving credits to all of you for the interesting discussion and comments. I agree with the general roadmap that was written by @recanman .

If @recanman thinks that PRs are ready to merge, I'll do it. In my opinion, I would wait to have all the CCS items completed, and then we can talk about separating packages. One thing that I must know and I'm really not aware of is how we should handle packaging monero-integrations-php for Wordpress, WHCMS, PrestaShop and others, where final users do not have the possibility to run composer init for lack of skills or resources.

Let me know if you have any questions

from monerophp.

I was literally just working on a PR this evening to the wp-cli/dist-archive-command project for packaging WordPress plugins. I also maintain a project for using Composer packages in WordPress plugins. Here's a nice GitHub Action that builds and attaches the release for a Bitcoin gateway I'm working on, and there are a few lines commented out which can push the release to the WordPress.org repository.

I want to get some E2E tests written against the existing WordPress plugin before I start changing anything! My long goal here has always been to contribute to the WordPress plugin.

from monerophp.

Thanks for the kind words everybody.

On a side note, I didn't really think about if I wanted to put my code under the monero-integrations umbrella, just wanted to let everybody know what I was working on, however off course my code can be used by anybody!

Regarding the separating of packages, is there any work already being done on a separate cryptography related package ?

@BrianHenryIE your work is also amazing!
About the tests, I'm not aware of how other projects do this, except monero-rpc-rs on which I based some of my tests, I have a couple tests running against a chain generated by the generateblocks method but that is fairly limited.

from monerophp.

I just opened a PR to the WordPress plugin to start adding tests: monero-integrations/monerowp#121

@lpoirothattermann That looks good. I found a post about running a local network – two instances of stagenet connected only to each other. I.e. a real Monero network but empty blockchain. I think maybe following those instructions with the one node might even be enough, I haven't tested yet – I'll pull your code and look into it soon.

And last week I asked for help getting Monero Explorer running on Docker: moneroexamples/onion-monero-blockchain-explorer#293

The xmrchain.net JSON API has been down for a few weeks, which isn't good news for the WordPress plugin: moneroexamples/onion-monero-blockchain-explorer#291

from monerophp.

@serhack Can you pin this issue maybe? And merge PRs please, to avoid people duplicating the same work.

https://docs.github.com/en/issues/tracking-your-work-with-issues/pinning-an-issue-to-your-repository

from monerophp.

I was waiting for all the PR made by @recanman to be ready for merging.

from monerophp.

I will close those PRs soon, as it seems that there has been a lot of overlapping of work. Other projects have emerged (refactor-ring's), and I think its best on focusing on getting that merged now. Getting #141, #142, and #143 merged is a good idea.

from monerophp.

@serhack Apologies for mentioning you again, but may you merge #141, #142, and #143?

from monerophp.

Well, everything has been merged. I see that refactor-ring's repository is now considered complete for the most part.
From what I see, it is now a matter of merging it into this repository and separating ed25519.php.

Do you have any recommendation on how to do this @refring?

from monerophp.

No response, that is ok.
There have not been any commits in a couple of months to refring's repository, so I'll begin rearranging and merging the repository into a branch and make a PR when I am free (maybe this weekend).

from monerophp.

Hi @recanman , sorry for not getting back to you sooner.

My library is pretty much complete, you're free to merge it into this project but I unfortunately don't really have the time currently to help with that.

from monerophp.

Thank you for the response.
I understand. I'm still trying to find time myself to complete the merge.

from monerophp.