-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Augmented submit fields should be included in sign-and-submit mode (Version: 1.5.0-b6) #3284
Comments
I think we should consider deprecating sign-and-submit mode, and create an issue for building a separate signing tool. The separate signing tool can be implemented in C++ or in TypeScript. (More diverse implementations is a good thing, in my opinion. We should start with C++ or TypeScript because the building blocks are already complete. I'd also be in favor of signing tools/libraries in other popular languages, like Python, Java, and Ruby, but these would require a lot more work.) |
Neither C++ nor TypeScript are easy to call from other languages (e.g. via CFFI) - until there's a solution that is as easy to use from any(!) language as JSON-RPC I'd like to keep this mode within |
I agree with @MarkusTeufelberger and I believe I can sum up the case in favor as follows: To submit transactions to the XRP Ledger, you need the following bare minimum:
Everything else is optional, but you absolutely need both of these things. Currently ¹ If your signing software screws up, any number of things can go wrong: not being able to send a transaction at a crucial time (invalid signature or blob format) is the most likely outcome, accidentally opening yourself up to losing all your money (leaking your secret key), suffering reputational damage, accidentally discharging funds (correct signature of incorrect transaction instructions) etc. ² You can, to a large extent, rely on "public" |
FWIW I find sign-and-submit mode very useful in standalone mode when writing new transaction types. If I want to sign with a library, I need to update the binary codec, which is annoying when I'm still testing things out and everything is in flux. |
I'd like to reiterate the security concerns highlighted by Nik in commit 38c3a46:
Instead of reading the secret in clear-text from command line, we could read it from a file. This reduces visibility to other users on a server, but it's still not foolproof. |
While we could do this, the command-line risks are a much smaller problem than transmitting a secret over the internet, potentially in plain-text, and potentially to an unknown / hostile server. |
Yes, that is true. This problem must have been solved in other contexts. For instance, we could mimic the ways of web browsers in handling and transferring secrets. That would still leave hostile servers as a possible attack vector. :/ |
Users should use public key cryptography the way it was intended to be used: the private key always remains private and should never be transmitted. Transaction signing can, and should, be performed locally. That said, sign-and-submit is still useful on trusted servers (i.e. you can use it on a server you solely control) and for testing on networks where the assets/tokens (and keys) have no value. |
Could sign-and-submit be switched to an admin-only command? I think the level of trust is roughly equivalent. |
In the An admin method places restrictions on who can invoke the Being a non-admin user, I've found this method very useful. It would be an inconvenience if we upgrade it to an admin method. |
How are you using it as a non-admin user? No public server supports it. |
I use it with my local running version of rippled. But I don't use sudo or any admin privileges. Maybe I'm still considered an admin user? I don't know. |
You're considered an admin user, then. It is likely automatically giving you the admin versions of the RPCs; if not, you can easily change the configuration to do that. https://xrpl.org/docs/tutorials/http-websocket-apis/build-apps/get-started/#admin-access |
thanks, I'll look into it |
Issue Description
The new fields added to the submit method response by #3125 are only provided when you submit a binary blob, not when you provide a JSON and secret.
The fields should be provided in all responses to the
submit
command.Steps to Reproduce
For a quick offline test:
Start rippled in standalone mode with a fresh ledger.
Sign-and-submit a payment from the genesis account to another address of your choice.
Expected Result
The new fields, such as
applied
,broadcast
,account_sequence_available
, etc. should be included in the response. Here's an example of a similar transaction submitted as binary, with the new fields provided:Actual Result
Actual result matches the old (v1.4.0 and lower) result fields for the submit method:
Environment
Version: rippled version 1.5.0-b6
OS: Arch Linux (latest) x86_64
Self-compiled, Boost 1.72.0. All unit tests pass.
The text was updated successfully, but these errors were encountered: