Skip to content

Commit 2ce3447

Browse files
vasildvasild
committed
Deduplicate the message verifying code
The logic of verifying a message was duplicated in 2 places: src/qt/signverifymessagedialog.cpp SignVerifyMessageDialog::on_verifyMessageButton_VM_clicked() src/rpc/misc.cpp verifymessage() with the only difference being the result handling. Move the logic into a dedicated src/util/message.cpp MessageVerify() which returns a set of result codes, call it from the 2 places and just handle the results differently in the callers.
1 parent 470664f commit 2ce3447

File tree

9 files changed

+211
-62
lines changed

9 files changed

+211
-62
lines changed

‎src/Makefile.am

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ BITCOIN_CORE_H = \
220220
util/system.h \
221221
util/macros.h \
222222
util/memory.h \
223+
util/message.h \
223224
util/moneystr.h \
224225
util/rbf.h \
225226
util/settings.h \
@@ -517,6 +518,7 @@ libbitcoin_util_a_SOURCES = \
517518
util/error.cpp \
518519
util/fees.cpp \
519520
util/system.cpp \
521+
util/message.cpp \
520522
util/moneystr.cpp \
521523
util/rbf.cpp \
522524
util/settings.cpp \

‎src/qt/signverifymessagedialog.cpp

Lines changed: 42 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
#include <qt/walletmodel.h>
1212

1313
#include <key_io.h>
14-
#include <util/validation.h> // For strMessageMagic
14+
#include <util/message.h> // For strMessageMagic, MessageVerify()
1515
#include <wallet/wallet.h>
1616

1717
#include <vector>
@@ -189,51 +189,57 @@ void SignVerifyMessageDialog::on_addressBookButton_VM_clicked()
189189

190190
void SignVerifyMessageDialog::on_verifyMessageButton_VM_clicked()
191191
{
192-
CTxDestination destination = DecodeDestination(ui->addressIn_VM->text().toStdString());
193-
if (!IsValidDestination(destination)) {
192+
const std::string& address = ui->addressIn_VM->text().toStdString();
193+
const std::string& signature = ui->signatureIn_VM->text().toStdString();
194+
const std::string& message = ui->messageIn_VM->document()->toPlainText().toStdString();
195+
196+
const auto result = MessageVerify(address, signature, message);
197+
198+
if (result == MessageVerificationResult::OK) {
199+
ui->statusLabel_VM->setStyleSheet("QLabel { color: green; }");
200+
} else {
194201
ui->statusLabel_VM->setStyleSheet("QLabel { color: red; }");
195-
ui->statusLabel_VM->setText(tr("The entered address is invalid.") + QString(" ") + tr("Please check the address and try again."));
196-
return;
197202
}
198-
if (!boost::get<PKHash>(&destination)) {
203+
204+
switch (result) {
205+
case MessageVerificationResult::OK:
206+
ui->statusLabel_VM->setText(
207+
QString("<nobr>") + tr("Message verified.") + QString("</nobr>")
208+
);
209+
return;
210+
case MessageVerificationResult::ERR_INVALID_ADDRESS:
211+
ui->statusLabel_VM->setText(
212+
tr("The entered address is invalid.") + QString(" ") +
213+
tr("Please check the address and try again.")
214+
);
215+
return;
216+
case MessageVerificationResult::ERR_ADDRESS_NO_KEY:
199217
ui->addressIn_VM->setValid(false);
200-
ui->statusLabel_VM->setStyleSheet("QLabel { color: red; }");
201-
ui->statusLabel_VM->setText(tr("The entered address does not refer to a key.") + QString(" ") + tr("Please check the address and try again."));
218+
ui->statusLabel_VM->setText(
219+
tr("The entered address does not refer to a key.") + QString(" ") +
220+
tr("Please check the address and try again.")
221+
);
202222
return;
203-
}
204-
205-
bool fInvalid = false;
206-
std::vector<unsigned char> vchSig = DecodeBase64(ui->signatureIn_VM->text().toStdString().c_str(), &fInvalid);
207-
208-
if (fInvalid)
209-
{
223+
case MessageVerificationResult::ERR_MALFORMED_SIGNATURE:
210224
ui->signatureIn_VM->setValid(false);
211-
ui->statusLabel_VM->setStyleSheet("QLabel { color: red; }");
212-
ui->statusLabel_VM->setText(tr("The signature could not be decoded.") + QString(" ") + tr("Please check the signature and try again."));
225+
ui->statusLabel_VM->setText(
226+
tr("The signature could not be decoded.") + QString(" ") +
227+
tr("Please check the signature and try again.")
228+
);
213229
return;
214-
}
215-
216-
CHashWriter ss(SER_GETHASH, 0);
217-
ss << strMessageMagic;
218-
ss << ui->messageIn_VM->document()->toPlainText().toStdString();
219-
220-
CPubKey pubkey;
221-
if (!pubkey.RecoverCompact(ss.GetHash(), vchSig))
222-
{
230+
case MessageVerificationResult::ERR_PUBKEY_NOT_RECOVERED:
223231
ui->signatureIn_VM->setValid(false);
224-
ui->statusLabel_VM->setStyleSheet("QLabel { color: red; }");
225-
ui->statusLabel_VM->setText(tr("The signature did not match the message digest.") + QString(" ") + tr("Please check the signature and try again."));
232+
ui->statusLabel_VM->setText(
233+
tr("The signature did not match the message digest.") + QString(" ") +
234+
tr("Please check the signature and try again.")
235+
);
226236
return;
227-
}
228-
229-
if (!(CTxDestination(PKHash(pubkey)) == destination)) {
230-
ui->statusLabel_VM->setStyleSheet("QLabel { color: red; }");
231-
ui->statusLabel_VM->setText(QString("<nobr>") + tr("Message verification failed.") + QString("</nobr>"));
237+
case MessageVerificationResult::ERR_NOT_SIGNED:
238+
ui->statusLabel_VM->setText(
239+
QString("<nobr>") + tr("Message verification failed.") + QString("</nobr>")
240+
);
232241
return;
233242
}
234-
235-
ui->statusLabel_VM->setStyleSheet("QLabel { color: green; }");
236-
ui->statusLabel_VM->setText(QString("<nobr>") + tr("Message verified.") + QString("</nobr>"));
237243
}
238244

239245
void SignVerifyMessageDialog::on_clearButton_VM_clicked()

‎src/rpc/misc.cpp

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@
1111
#include <rpc/util.h>
1212
#include <script/descriptor.h>
1313
#include <util/check.h>
14+
#include <util/message.h> // For strMessageMagic, MessageVerify()
1415
#include <util/strencodings.h>
1516
#include <util/system.h>
16-
#include <util/validation.h>
1717

1818
#include <stdint.h>
1919
#include <tuple>
@@ -276,31 +276,21 @@ static UniValue verifymessage(const JSONRPCRequest& request)
276276
std::string strSign = request.params[1].get_str();
277277
std::string strMessage = request.params[2].get_str();
278278

279-
CTxDestination destination = DecodeDestination(strAddress);
280-
if (!IsValidDestination(destination)) {
279+
switch (MessageVerify(strAddress, strSign, strMessage)) {
280+
case MessageVerificationResult::ERR_INVALID_ADDRESS:
281281
throw JSONRPCError(RPC_TYPE_ERROR, "Invalid address");
282-
}
283-
284-
const PKHash *pkhash = boost::get<PKHash>(&destination);
285-
if (!pkhash) {
282+
case MessageVerificationResult::ERR_ADDRESS_NO_KEY:
286283
throw JSONRPCError(RPC_TYPE_ERROR, "Address does not refer to key");
287-
}
288-
289-
bool fInvalid = false;
290-
std::vector<unsigned char> vchSig = DecodeBase64(strSign.c_str(), &fInvalid);
291-
292-
if (fInvalid)
284+
case MessageVerificationResult::ERR_MALFORMED_SIGNATURE:
293285
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Malformed base64 encoding");
294-
295-
CHashWriter ss(SER_GETHASH, 0);
296-
ss << strMessageMagic;
297-
ss << strMessage;
298-
299-
CPubKey pubkey;
300-
if (!pubkey.RecoverCompact(ss.GetHash(), vchSig))
286+
case MessageVerificationResult::ERR_PUBKEY_NOT_RECOVERED:
287+
case MessageVerificationResult::ERR_NOT_SIGNED:
301288
return false;
289+
case MessageVerificationResult::OK:
290+
return true;
291+
}
302292

303-
return (pubkey.GetID() == *pkhash);
293+
return false;
304294
}
305295

306296
static UniValue signmessagewithprivkey(const JSONRPCRequest& request)

‎src/test/util_tests.cpp

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <sync.h>
1010
#include <test/util/setup_common.h>
1111
#include <test/util/str.h>
12+
#include <util/message.h> // For MessageVerify()
1213
#include <util/moneystr.h>
1314
#include <util/strencodings.h>
1415
#include <util/string.h>
@@ -2025,4 +2026,56 @@ BOOST_AUTO_TEST_CASE(test_tracked_vector)
20252026
BOOST_CHECK_EQUAL(v8[2].copies, 0);
20262027
}
20272028

2029+
BOOST_AUTO_TEST_CASE(message_verify)
2030+
{
2031+
BOOST_CHECK_EQUAL(
2032+
MessageVerify(
2033+
"invalid address",
2034+
"signature should be irrelevant",
2035+
"message too"),
2036+
MessageVerificationResult::ERR_INVALID_ADDRESS);
2037+
2038+
BOOST_CHECK_EQUAL(
2039+
MessageVerify(
2040+
"3B5fQsEXEaV8v6U3ejYc8XaKXAkyQj2MjV",
2041+
"signature should be irrelevant",
2042+
"message too"),
2043+
MessageVerificationResult::ERR_ADDRESS_NO_KEY);
2044+
2045+
BOOST_CHECK_EQUAL(
2046+
MessageVerify(
2047+
"1KqbBpLy5FARmTPD4VZnDDpYjkUvkr82Pm",
2048+
"invalid signature, not in base64 encoding",
2049+
"message should be irrelevant"),
2050+
MessageVerificationResult::ERR_MALFORMED_SIGNATURE);
2051+
2052+
BOOST_CHECK_EQUAL(
2053+
MessageVerify(
2054+
"1KqbBpLy5FARmTPD4VZnDDpYjkUvkr82Pm",
2055+
"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=",
2056+
"message should be irrelevant"),
2057+
MessageVerificationResult::ERR_PUBKEY_NOT_RECOVERED);
2058+
2059+
BOOST_CHECK_EQUAL(
2060+
MessageVerify(
2061+
"15CRxFdyRpGZLW9w8HnHvVduizdL5jKNbs",
2062+
"IPojfrX2dfPnH26UegfbGQQLrdK844DlHq5157/P6h57WyuS/Qsl+h/WSVGDF4MUi4rWSswW38oimDYfNNUBUOk=",
2063+
"I never signed this"),
2064+
MessageVerificationResult::ERR_NOT_SIGNED);
2065+
2066+
BOOST_CHECK_EQUAL(
2067+
MessageVerify(
2068+
"15CRxFdyRpGZLW9w8HnHvVduizdL5jKNbs",
2069+
"IPojfrX2dfPnH26UegfbGQQLrdK844DlHq5157/P6h57WyuS/Qsl+h/WSVGDF4MUi4rWSswW38oimDYfNNUBUOk=",
2070+
"Trust no one"),
2071+
MessageVerificationResult::OK);
2072+
2073+
BOOST_CHECK_EQUAL(
2074+
MessageVerify(
2075+
"11canuhp9X2NocwCq7xNrQYTmUgZAnLK3",
2076+
"IIcaIENoYW5jZWxsb3Igb24gYnJpbmsgb2Ygc2Vjb25kIGJhaWxvdXQgZm9yIGJhbmtzIAaHRtbCeDZINyavx14=",
2077+
"Trust me"),
2078+
MessageVerificationResult::OK);
2079+
}
2080+
20282081
BOOST_AUTO_TEST_SUITE_END()

‎src/util/message.cpp

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Copyright (c) 2009-2010 Satoshi Nakamoto
2+
// Copyright (c) 2009-2020 The Bitcoin Core developers
3+
// Distributed under the MIT software license, see the accompanying
4+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
5+
6+
#include <hash.h> // For CHashWriter
7+
#include <key_io.h> // For DecodeDestination()
8+
#include <pubkey.h> // For CPubKey
9+
#include <script/standard.h> // For CTxDestination, IsValidDestination(), PKHash
10+
#include <serialize.h> // For SER_GETHASH
11+
#include <util/message.h>
12+
#include <util/strencodings.h> // For DecodeBase64()
13+
14+
#include <string>
15+
#include <vector>
16+
17+
const std::string strMessageMagic = "Bitcoin Signed Message:\n";
18+
19+
MessageVerificationResult MessageVerify(
20+
const std::string& address,
21+
const std::string& signature,
22+
const std::string& message)
23+
{
24+
CTxDestination destination = DecodeDestination(address);
25+
if (!IsValidDestination(destination)) {
26+
return MessageVerificationResult::ERR_INVALID_ADDRESS;
27+
}
28+
29+
if (boost::get<PKHash>(&destination) == nullptr) {
30+
return MessageVerificationResult::ERR_ADDRESS_NO_KEY;
31+
}
32+
33+
bool invalid = false;
34+
std::vector<unsigned char> signature_bytes = DecodeBase64(signature.c_str(), &invalid);
35+
if (invalid) {
36+
return MessageVerificationResult::ERR_MALFORMED_SIGNATURE;
37+
}
38+
39+
CHashWriter ss(SER_GETHASH, 0);
40+
ss << strMessageMagic;
41+
ss << message;
42+
43+
CPubKey pubkey;
44+
if (!pubkey.RecoverCompact(ss.GetHash(), signature_bytes)) {
45+
return MessageVerificationResult::ERR_PUBKEY_NOT_RECOVERED;
46+
}
47+
48+
if (!(CTxDestination(PKHash(pubkey)) == destination)) {
49+
return MessageVerificationResult::ERR_NOT_SIGNED;
50+
}
51+
52+
return MessageVerificationResult::OK;
53+
}

‎src/util/message.h

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright (c) 2009-2010 Satoshi Nakamoto
2+
// Copyright (c) 2009-2020 The Bitcoin Core developers
3+
// Distributed under the MIT software license, see the accompanying
4+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
5+
6+
#ifndef BITCOIN_UTIL_MESSAGE_H
7+
#define BITCOIN_UTIL_MESSAGE_H
8+
9+
#include <string>
10+
11+
extern const std::string strMessageMagic;
12+
13+
/** The result of a signed message verification.
14+
* Message verification takes as an input:
15+
* - address (with whose private key the message is supposed to have been signed)
16+
* - signature
17+
* - message
18+
*/
19+
enum class MessageVerificationResult {
20+
//! The provided address is invalid.
21+
ERR_INVALID_ADDRESS,
22+
23+
//! The provided address is valid but does not refer to a public key.
24+
ERR_ADDRESS_NO_KEY,
25+
26+
//! The provided signature couldn't be parsed (maybe invalid base64).
27+
ERR_MALFORMED_SIGNATURE,
28+
29+
//! A public key could not be recovered from the provided signature and message.
30+
ERR_PUBKEY_NOT_RECOVERED,
31+
32+
//! The message was not signed with the private key of the provided address.
33+
ERR_NOT_SIGNED,
34+
35+
//! The message verification was successful.
36+
OK
37+
};
38+
39+
/** Verify a signed message.
40+
* @param[in] address Signer's bitcoin address, it must refer to a public key.
41+
* @param[in] signature The signature in base64 format.
42+
* @param[in] message The message that was signed.
43+
* @return result code */
44+
MessageVerificationResult MessageVerify(
45+
const std::string& address,
46+
const std::string& signature,
47+
const std::string& message);
48+
49+
#endif // BITCOIN_UTIL_MESSAGE_H

‎src/util/validation.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,3 @@ std::string FormatStateMessage(const ValidationState &state)
2121

2222
return state.GetRejectReason();
2323
}
24-
25-
const std::string strMessageMagic = "Bitcoin Signed Message:\n";

‎src/util/validation.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,4 @@ class ValidationState;
1313
/** Convert ValidationState to a human-readable message for logging */
1414
std::string FormatStateMessage(const ValidationState &state);
1515

16-
extern const std::string strMessageMagic;
17-
1816
#endif // BITCOIN_UTIL_VALIDATION_H

‎src/wallet/rpcwallet.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@
1919
#include <script/sign.h>
2020
#include <util/bip32.h>
2121
#include <util/fees.h>
22+
#include <util/message.h> // For strMessageMagic
2223
#include <util/moneystr.h>
2324
#include <util/string.h>
2425
#include <util/system.h>
2526
#include <util/url.h>
26-
#include <util/validation.h>
2727
#include <wallet/coincontrol.h>
2828
#include <wallet/feebumper.h>
2929
#include <wallet/psbtwallet.h>

0 commit comments

Comments
 (0)