-
Notifications
You must be signed in to change notification settings - Fork 990
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
stick to e=H(R|P|m) when use schnorr signature #2200
Conversation
Tested on Floonet. Ready for review. It will impact the wallet communication between the old version and this new version, if user use |
to make thing simple, I split those modification for message signature verification into an independent PR: #2203, and make this PR pure for the schnorr signature fixing. |
I feel nobody is using the optional message for the wallet send, so perhaps it's good time to merge this, before some use cases start using it, and avoid future compatibility problem. |
To be honest, I'm not really convinced this is necessary here... embedding the public key in the message is needed at transaction time to support the particulars of a Grin exchange, but when it's just simply a single signer signing a simple message there's no need to do anything special with it, hence the simple signature. Why does this need to be more complicated? |
I believe confirming to Gregory Maxwell,Andrew Poelstra,Yannick Seurin, and Peter Wuille 's multi-signature scheme is our best choice, since we already switched to it in our 2-of-2 signature for transaction kernel since And I'm afraid that Two words modification in this PR should not be a complicated change, right? 😄 So, if no any bad point in this change (I believe no), I would prefer this should be taken before mainnet. |
It might mean that any other services that want to verify the message would likely need to use our aggsig libs as opposed to other implementations, but might enable multi-signer messages. But not particularly worried either way in the absence of further info, so happy enough to merge this (will do it into re-floonet so it's picked up there) |
* Fix secondary scaling bugs; rename is_testnet -> is_floonet (#2215) * add global::is_mainnet() * use it to change pre-genesis pow type * rename is_testnet -> is_floonet * Support multiple chain configurations (#2217) * Support multiple chain configurations Supports generating the proper configuration for each chain type (mainnet, floonet, usernet). Will run them by default under their respective root directory (~/.grin/main, ~/.grin/floo, etc). Assigned default ports for mainnet, overriding them to keep Floonet ports unchanged. For now, starting on mainnet will abort. * Fixed usernet command line help message. Fixes #2217 * Differing magic numbers for each chain type (#2208) * stick to e=H(R|P|m) when use schnorr signature (#2200) * stick to e=H(R|P|m) when use schnorr signature * (1)add verify_slate_messages for wallet receive (2)log the message content * remove debug log on verify_slate_messages * verify the sender's message signature when receive_tx in wallet listen * Revert "remove debug log on verify_slate_messages" This reverts commit 65ea32a. * Revert "rustfmt" This reverts commit c380ab9. * Revert "(1)add verify_slate_messages for wallet receive (2)log the message content" This reverts commit 9584ca7. * [re-floonet] Keychain Floonet BIP32 version/network option (#2235) * add 'is_floonet' property to keychain * fix hex encoding and tests * Fix couple floonet loose ends (#2230) * Fix couple floonet loose ends. Fixes #2216 * Doc fix for sig message * Refuse unkown kernel features (#2244) * Minor: magic number change for re-floonet * Set pre genesis is_secondary to true (#2247) * Minor: tx validation error display underlying * New floonet genesis * genesis rustfmt * Use chain-specific config for wallet toml gen * Fix default wallet_listener_url * New more reasonable genesis block, bumped version * genesis rustfmt * Couple minor fixes to genesis generation script
happen to see the message signature when wallet sending is still using the old form with
e=Hash(R|m)
, better to stick to the formal form everywhere.Note: it's not a consensus breaking issue, but once it's modified and some nodes not updated, the wallet sending between current version and new version will fail because of message signature verification fail.
DNM it because I can ONLY test this PR after my coinbase mature (tomorrow afternoon).