feat(funding-service): add server and status endpoint - #2600
feat(funding-service): add server and status endpoint#2600SantiagoPittella wants to merge 7 commits into
Conversation
f41b0f2 to
1be9fc9
Compare
| "the funding account {} is not public: the service reads its state from the node, \ | ||
| which only stores the full state of a public account", |
There was a problem hiding this comment.
Perhaps a question whether we should be doing this or not 🤔 I guess the value owned would be small..
There was a problem hiding this comment.
One thing about limiting the service to public accounts is that it simplifies operations. The service does not even need to keep track of the account itself, just the private key. So I would say if the service is limited to public accounts only, scrap any functionality that tries to keep and validate the local state of the account versus the state provided by the network, and just rely on the latter. Only if the service supports private accounts should you take care of keeping the local state valid.
There was a problem hiding this comment.
It does simplify things, also means it easy to check if its out of funds via the explorer.
There was a problem hiding this comment.
One thing I'm wondering about is whether gRPC is overkill and in general is the right interface for this service. I get that we use gRPC throughout the whole stack, but this feels like something that may be consumed directly by apps/frontends, and the API seems simple enough that plain HTTP/JSON could make integration quite a bit easier.
For example, if the wallet test harness (which is quite complete in terms of what it integration-tests so we probably want to take it into account here) wanted to use this, an HTTP endpoint could just be called directly from the extension with fetch, and the returned note bytes could be parsed and consumed in a very straightforward way. With gRPC we'd need all the corresponding gRPC-Web/client plumbing and generated bindings for this service. Maybe this is not so bad either but it definitely increases complexity.
Maybe I'm overthinking it, but it seems worth considering whether we want this service to have a simpler, more generic interface before committing to gRPC. cc @Mirko-von-Leipzig
| "the funding account {} is not public: the service reads its state from the node, \ | ||
| which only stores the full state of a public account", |
There was a problem hiding this comment.
One thing about limiting the service to public accounts is that it simplifies operations. The service does not even need to keep track of the account itself, just the private key. So I would say if the service is limited to public accounts only, scrap any functionality that tries to keep and validate the local state of the account versus the state provided by the network, and just rely on the latter. Only if the service supports private accounts should you take care of keeping the local state valid.
1be9fc9 to
e2256d4
Compare
I thought of grpc because of two things: In general, I'm not aware into the wallet integration tests impl itself, it was coherent to use gRPC because this is what our services in general uses. And in particular, I had in mind the client integration tests and the network-monitor as the primary users of this service, so it felt natural to use gRPC |
I don't have a strong opinion, I initially thought a simple A benefit of the HTTP style is that its much simpler for scripts and a human to use e.g. if we need to send funds somewhere for some random account. |
Okok, sounds better. I'm moving to HTTP |
1e349ad to
7c5337e
Compare
Summary
Adds
miden-funding-service, a binary that owns one wallet account holding the chain's native asset and will send that asset to any account that asks for it.The funding account is a genesis [[wallet]]. To let the service load it from a fixed path, a wallet entry can now take a name, which writes its account file to .mac instead of wallet_.mac. Duplicate names are rejected before anything is written.
This PR only includes its status endpoint and plumbing.
Changelog