-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Experimental feature: remote proving service #2120
Conversation
Side note: I expect we will move, rename, or remove |
Renamed the binaries and added a conditional compilation flag. |
3436993
to
b94d175
Compare
Adjusted the wire protocol to take a list of |
☔ The latest upstream changes (presumably #1636) made this pull request unmergeable. Please resolve the merge conflicts. |
@nathan-at-least as-is, this protocol still enables DP-Z with trusted proving services. I know you wanted what we land as DP-T to not support trusted proving services, but given that there will be a client library in front, is it sufficient for that library to provide a DP-T-only API? |
☔ The latest upstream changes (presumably #2183) made this pull request unmergeable. Please resolve the merge conflicts. |
@nathan-at-least I can't remember whether you wanted to bump this from 1.0.8; if yes, then the merge conflict above should resolve itself once the |
@arcalinea and I bumped this from 1.0.8, and @nathan-at-least concurred on #zcash-dev. As far as I understand, that commit (802ea76) will not be reverted for 1.0.9, but payment offloading is in the 1.0.9 milestone, so this conflict does need to be resolved. |
configure.ac
Outdated
@@ -203,6 +203,12 @@ AC_ARG_WITH([daemon], | |||
[build_bitcoind=$withval], | |||
[build_bitcoind=yes]) | |||
|
|||
AC_ARG_WITH([proving-service], | |||
[AS_HELP_STRING([--with-proving-service], | |||
[build zcash-proving-service (default is no)])], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking: Just spotted this should be default=no
😝
Rebased on master. |
Addressed comments by @arielgabizon and @bitcartel. |
@zkbot retry |
@zkbot try |
Experimental feature: remote proving service This implements proving service support in `z_sendmany` and adds two new binaries: - `zcash-proving-service`: listens on (one or both of) ZMQ and WebSocket ports for witness data, and returns the corresponding proof. - `ExampleProvingServiceClient` (in `src/zcash`): creates five new `JSDescription`s using the proving service to calculate the proofs, and validates each proof. To build: `CONFIGURE_FLAGS="--with-proving-service-daemon" ./zcutil/build.sh` This is an experimental feature, and the network protocol is likely to change. Until the protocol has been specified in a ZIP, do not rely on it for interoperation. Part of #1113. Closes #2066.
☀️ Test successful - pr-try |
I'm not getting the |
@arielgabizon the file should be generated as |
Unfortunately, this still does not have any ACKs, and there are now only three work days until the 1.0.14 release, so I'm bumping this to 1.0.15. |
I should be able to ack today/monday, if I manage to run the code. |
src/zcash/JoinSplit.hpp
Outdated
phi(phi), rt(rt), h_sig(h_sig), | ||
inputs(inputs), outputs(outputs), | ||
vpub_old(vpub_old), vpub_new(vpub_new) { } | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this being an explicit class, I think it makes things clearer.
@@ -59,7 +83,7 @@ class JoinSplit { | |||
const uint256& pubKeyHash | |||
); | |||
|
|||
virtual ZCProof prove( | |||
virtual JSProofWitness<NumInputs, NumOutputs> witness( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No opinion. The spec doesn't have a name for this (it's not the same as the primary input or the auxiliary input, because those have more redundancy).
# Copying and distribution of this file, with or without modification, are | ||
# permitted in any medium without royalty provided the copyright notice | ||
# and this notice are preserved. This file is offered as-is, without any | ||
# warranty. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add this to contrib/debian/copyright:
Files: build-aux/m4/ax_boost_regex.m4
Copyright: 2008 Thomas Porschberg <thomas@randspringer.de>;
2008 Michael Tindal
License: GNU-All-permissive-License
Actually there should be similar entries for all of the files under build-aux/m4:
Files: build-aux/m4/ax_boost_base.m4
Copyright: 2008 Thomas Porschberg <thomas@randspringer.de>;
2009 Peter Adolphs
License: GNU-All-permissive-License
Files: build-aux/m4/ax_boost_chrono.m4
Copyright: 2012 Xiyue Deng <manphiz@gmail.com>
License: GNU-All-permissive-License
Files: build-aux/m4/ax_boost_filesystem.m4
Copyright: 2008 Thomas Porschberg <thomas@randspringer.de>;
2009 Michael Tindal;
2009 Roman Rybalko <libtorrent@romanr.info>
License: GNU-All-permissive-License
Files: build-aux/m4/ax_boost_program_options.m4;
build-aux/m4/ax_boost_unit_test_framework.m4
Copyright: 2008 Thomas Porschberg <thomas@randspringer.de>
License: GNU-All-permissive-License
Files: build-aux/m4/ax_boost_system.m4
Copyright: 2008 Thomas Porschberg <thomas@randspringer.de>;
2008 Michael Tindal;
2008 Daniel Casimiro <dan.casimiro@gmail.com>
License: GNU-All-permissive-License
Files: build-aux/m4/ax_cxx_compile_stdcxx.m4
Copyright: 2008 Benjamin Kosnik <bkoz@redhat.com>;
2012 Zack Weinberg <zackw@panix.com>;
2013 Roy Stogner <roystgnr@ices.utexas.edu>;
2014, 2015 Google Inc. contributed by Alexey Sokolov <sokolov@google.com>;
2015 Paul Norman <penorman@mac.com>;
2015 Moritz Klammler <moritz@klammler.eu>
License: GNU-All-permissive-License
Files: build-aux/m4/ax_gcc_func_attribute.m4
Copyright: 2013 Gabriele Svelto <gabriele.svelto@gmail.com>
License: GNU-All-permissive-License
Files: build-aux/m4/l_atomic.m4
Copyright: 2004-2017 Tim Kosse
License: GPLv2
Files: build-aux/m4/ax_check_compile_flag.m4;
build-aux/m4/ax_check_link_flag.m4;
build-aux/m4/ax_check_preproc_flag.m4
Copyright: 2008 Guido U. Draheim <guidod@gmx.de>;
2011 Maarten Bosmans <mkbosmans@gmail.com>
License: GPLv3-with-Autoconf-Macro-exception
Files: build-aux/m4/ax_openmp.m4
Copyright: 2008 Steven G. Johnson <stevenj@alum.mit.edu>;
2015 John W. Peterson <jwpeterson@gmail.com>;
2016 Nick R. Papior <nickpapior@gmail.com>
License: GPLv3-with-Autoconf-Macro-exception
Files: build-aux/m4/ax_pthread.m4
Copyright: 2008 Steven G. Johnson <stevenj@alum.mit.edu>;
2011 Daniel Richard G. <skunk@iSKUNK.ORG>
License: GPLv3-with-Autoconf-Macro-exception
License: GPLv3-with-Autoconf-Macro-exception
This program is free software: you can redistribute it and/or modify it
under the terms of the GNU General Public License as published by the
Free Software Foundation, either version 3 of the License, or (at your
option) any later version.
.
This program is distributed in the hope that it will be useful, but
WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
See the GNU General Public License for more details.
.
You should have received a copy of the GNU General Public License along
with this program. If not, see <http://www.gnu.org/licenses/>.
.
As a special exception, the respective Autoconf Macro's copyright owner
gives unlimited permission to copy, distribute and modify the configure
scripts that are the output of Autoconf when processing the Macro. You
need not follow the terms of the GNU General Public License when using
or distributing such scripts, even though portions of the text of the
Macro appear in them. The GNU General Public License (GPL) does govern
all other use of the material that constitutes the Autoconf Macro.
.
This special exception to the GPL applies to versions of the Autoconf
Macro released by the Autoconf Archive. When you make and distribute a
modified version of the Autoconf Macro, you may extend this special
exception to the GPL to apply to your modified version as well.
build-aux/m4/l_atomic.m4 comes from Filezilla which is GPLv2. This is a problem, which I will file another ticket for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That other ticket is #2827.
$(package)_download_path=https://github.com/zeromq/$(package)/archive/ | ||
$(package)_file_name=$(package)-$($(package)_version).tar.gz | ||
$(package)_download_file=v$($(package)_version).tar.gz | ||
$(package)_sha256_hash=c204c731bcb7810ca3a2c5515e88974ef2ff8d0589e60a897dc238b369180e7b |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may be misinterpreting, but this recently fixed ticket looks like a memory safety bug: zeromq/azmq#111
Not to worry, that was before the 1.0 release.
$(package)_download_path=https://github.com/zeromq/$(package)/archive/ | ||
$(package)_file_name=$(package)-$($(package)_version).tar.gz | ||
$(package)_download_file=v$($(package)_version).tar.gz | ||
$(package)_sha256_hash=c204c731bcb7810ca3a2c5515e88974ef2ff8d0589e60a897dc238b369180e7b |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zeromq/azmq#129 is a reliability bug.
src/primitives/transaction.cpp
Outdated
@@ -22,7 +22,7 @@ JSDescription::JSDescription(ZCJoinSplit& params, | |||
{ | |||
boost::array<libzcash::Note, ZC_NUM_JS_OUTPUTS> notes; | |||
|
|||
auto witness = params.witness( | |||
witness = params.witness( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this change make the witness stay in memory longer (than needed)?
src/zcash-proving-service.cpp
Outdated
|
||
using namespace libzcash; | ||
|
||
std::string DEFAULT_ZMQ_BIND_ADDRESS = "tcp://*:8234"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this binding to all interfaces? Do we want to do that by default?
|
||
zmq::message_t reply; | ||
LogPrint("zrpc", "%s: Waiting for proofs…\n", log_id); | ||
// Add 5% margin to timeout for overhead |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this enough margin? I'd suggest 10% plus 5 seconds.
&esk); // parameter expects pointer to esk, so pass in address | ||
{ | ||
|
||
if (generateProof) { | ||
auto verifier = libzcash::ProofVerifier::Strict(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was that fixed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
From what I can tell, this doesn't support trustless (i.e. without having the private keys) Z proving, right? Is this still a limitation in Sapling or could this be updated to support it after Sapling activates? Edit: Disregard this, I have found the DP-Z proposal here #2171 (comment) 😄 Edit 2: On second thoughts, that looks like an abandoned feature? |
This is definitely an abandoned feature; Sapling is fast enough that this isn't needed in practice, and if it were in some context (which likely wouldn't be A fun experiment, consigned to the dustbin of history 😄 |
This implements proving service support in
z_sendmany
and adds two new binaries:zcash-proving-service
: listens on (one or both of) ZMQ and WebSocket ports for witness data, and returns the corresponding proof.ExampleProvingServiceClient
(insrc/zcash
): creates five newJSDescription
s using the proving service to calculate the proofs, and validates each proof.To build:
CONFIGURE_FLAGS="--with-proving-service-daemon" ./zcutil/build.sh
This is an experimental feature, and the network protocol is likely to change. Until the protocol has been specified in a ZIP, do not rely on it for interoperation.
Part of #1113. Closes #2066.