Skip to content

Commit 32dca0f

Browse files
kjozwiakkjozwiak
authored
Merge pull request #11277 from brave/revert-11257-pr11206_sign_typed_data_1.32.x
Revert "Sign typed data (uplift to 1.32.x) "
2 parents 003fee7 + e31edc3 commit 32dca0f

20 files changed

+36
-1320
lines changed

‎browser/brave_wallet/brave_wallet_provider_impl_unittest.cc

-141
Original file line numberDiff line numberDiff line change
@@ -358,35 +358,6 @@ class BraveWalletProviderImplUnitTest : public testing::Test {
358358
run_loop.Run();
359359
}
360360

361-
void SignTypedMessage(absl::optional<bool> user_approved,
362-
const std::string& address,
363-
const std::string& message,
364-
const std::string& message_to_sign,
365-
base::Value&& domain,
366-
std::string* signature_out,
367-
int* error_out,
368-
std::string* error_message_out) {
369-
if (!signature_out || !error_out || !error_message_out)
370-
return;
371-
372-
base::RunLoop run_loop;
373-
provider()->SignTypedMessage(
374-
address, message, message_to_sign, std::move(domain),
375-
base::BindLambdaForTesting([&](const std::string& signature, int error,
376-
const std::string& error_message) {
377-
*signature_out = signature;
378-
*error_out = error;
379-
*error_message_out = error_message;
380-
run_loop.Quit();
381-
}));
382-
// Wait for BraveWalletProviderImpl::ContinueSignMessage
383-
browser_task_environment_.RunUntilIdle();
384-
if (user_approved)
385-
brave_wallet_service_->NotifySignMessageRequestProcessed(
386-
*user_approved, provider()->sign_message_id_ - 1);
387-
run_loop.Run();
388-
}
389-
390361
// current request id will be returned
391362
int SignMessageRequest(const std::string& address,
392363
const std::string& message) {
@@ -949,118 +920,6 @@ TEST_F(BraveWalletProviderImplUnitTest, SignMessage) {
949920
base::ASCIIToUTF16(addresses[0])));
950921
}
951922

952-
TEST_F(BraveWalletProviderImplUnitTest, SignTypedMessage) {
953-
EXPECT_EQ(eth_json_rpc_controller()->GetChainId(), "0x1");
954-
CreateWallet();
955-
AddAccount();
956-
const std::string valid_message_to_sign =
957-
"be609aee343fb3c4b28e1df9e632fca64fcfaede20f02e86244efddf30957bd2";
958-
std::string signature;
959-
int error;
960-
std::string error_message;
961-
base::Value domain(base::Value::Type::DICTIONARY);
962-
domain.SetIntKey("chainId", 1);
963-
SignTypedMessage(absl::nullopt, "1234", "{...}", valid_message_to_sign,
964-
domain.Clone(), &signature, &error, &error_message);
965-
EXPECT_TRUE(signature.empty());
966-
EXPECT_EQ(error, static_cast<int>(ProviderErrors::kInvalidParams));
967-
EXPECT_EQ(error_message,
968-
l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
969-
970-
SignTypedMessage(absl::nullopt, "0x12345678", "{...}", valid_message_to_sign,
971-
domain.Clone(), &signature, &error, &error_message);
972-
EXPECT_TRUE(signature.empty());
973-
EXPECT_EQ(error, static_cast<int>(ProviderErrors::kInvalidParams));
974-
EXPECT_EQ(error_message,
975-
l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
976-
977-
const std::string address = "0x1234567890123456789012345678901234567890";
978-
// domain not dict
979-
SignTypedMessage(absl::nullopt, address, "{...}", valid_message_to_sign,
980-
base::Value("not dict"), &signature, &error, &error_message);
981-
EXPECT_TRUE(signature.empty());
982-
EXPECT_EQ(error, static_cast<int>(ProviderErrors::kInvalidParams));
983-
EXPECT_EQ(error_message,
984-
l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
985-
986-
// not valid hex
987-
SignTypedMessage(absl::nullopt, address, "{...}", "brave", domain.Clone(),
988-
&signature, &error, &error_message);
989-
EXPECT_TRUE(signature.empty());
990-
EXPECT_EQ(error, static_cast<int>(ProviderErrors::kInvalidParams));
991-
EXPECT_EQ(error_message,
992-
l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
993-
994-
// not valid eip712 hash
995-
SignTypedMessage(absl::nullopt, address, "{...}", "deadbeef", domain.Clone(),
996-
&signature, &error, &error_message);
997-
EXPECT_TRUE(signature.empty());
998-
EXPECT_EQ(error, static_cast<int>(ProviderErrors::kInvalidParams));
999-
EXPECT_EQ(error_message,
1000-
l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
1001-
1002-
domain.SetIntKey("chainId", 4);
1003-
std::string chain_id = "0x4";
1004-
// not active network
1005-
SignTypedMessage(absl::nullopt, address, "{...}", valid_message_to_sign,
1006-
domain.Clone(), &signature, &error, &error_message);
1007-
EXPECT_TRUE(signature.empty());
1008-
EXPECT_EQ(error, static_cast<int>(ProviderErrors::kInternalError));
1009-
EXPECT_EQ(error_message,
1010-
l10n_util::GetStringFUTF8(
1011-
IDS_BRAVE_WALLET_SIGN_TYPED_MESSAGE_CHAIN_ID_MISMATCH,
1012-
base::ASCIIToUTF16(chain_id)));
1013-
domain.SetIntKey("chainId", 1);
1014-
1015-
SignTypedMessage(absl::nullopt, address, "{...}", valid_message_to_sign,
1016-
domain.Clone(), &signature, &error, &error_message);
1017-
EXPECT_TRUE(signature.empty());
1018-
EXPECT_EQ(error, static_cast<int>(ProviderErrors::kUnauthorized));
1019-
EXPECT_EQ(error_message,
1020-
l10n_util::GetStringFUTF8(IDS_WALLET_ETH_SIGN_NOT_AUTHED,
1021-
base::ASCIIToUTF16(address)));
1022-
1023-
// No permission
1024-
const std::vector<std::string> addresses = GetAddresses();
1025-
ASSERT_FALSE(address.empty());
1026-
SignTypedMessage(absl::nullopt, addresses[0], "{...}", valid_message_to_sign,
1027-
domain.Clone(), &signature, &error, &error_message);
1028-
EXPECT_TRUE(signature.empty());
1029-
EXPECT_EQ(error, static_cast<int>(ProviderErrors::kUnauthorized));
1030-
EXPECT_EQ(error_message,
1031-
l10n_util::GetStringFUTF8(IDS_WALLET_ETH_SIGN_NOT_AUTHED,
1032-
base::ASCIIToUTF16(addresses[0])));
1033-
GURL url("https://brave.com");
1034-
Navigate(url);
1035-
AddEthereumPermission(url);
1036-
SignTypedMessage(true, addresses[0], "{...}", valid_message_to_sign,
1037-
domain.Clone(), &signature, &error, &error_message);
1038-
1039-
EXPECT_FALSE(signature.empty());
1040-
EXPECT_EQ(error, 0);
1041-
EXPECT_TRUE(error_message.empty());
1042-
1043-
// User reject request
1044-
SignTypedMessage(false, addresses[0], "{...}", valid_message_to_sign,
1045-
domain.Clone(), &signature, &error, &error_message);
1046-
EXPECT_TRUE(signature.empty());
1047-
EXPECT_EQ(error, static_cast<int>(ProviderErrors::kUserRejectedRequest));
1048-
EXPECT_EQ(error_message,
1049-
l10n_util::GetStringUTF8(IDS_WALLET_USER_REJECTED_REQUEST));
1050-
1051-
keyring_controller()->Lock();
1052-
1053-
// nullopt for the first param here because we don't AddSignMessageRequest
1054-
// whent here are no accounts returned.
1055-
SignTypedMessage(absl::nullopt, addresses[0], "{...}", valid_message_to_sign,
1056-
domain.Clone(), &signature, &error, &error_message);
1057-
EXPECT_TRUE(signature.empty());
1058-
EXPECT_EQ(error, static_cast<int>(ProviderErrors::kUnauthorized));
1059-
EXPECT_EQ(error_message,
1060-
l10n_util::GetStringFUTF8(IDS_WALLET_ETH_SIGN_NOT_AUTHED,
1061-
base::ASCIIToUTF16(addresses[0])));
1062-
}
1063-
1064923
TEST_F(BraveWalletProviderImplUnitTest, SignMessageRequestQueue) {
1065924
CreateWallet();
1066925
AddAccount();

‎components/brave_wallet/browser/brave_wallet_provider_impl.cc

+12-55
Original file line numberDiff line numberDiff line change
@@ -322,63 +322,18 @@ void BraveWalletProviderImpl::SignMessage(const std::string& address,
322322
l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
323323
return;
324324
}
325-
326-
std::string message_str(message_bytes.begin(), message_bytes.end());
327-
if (!base::IsStringUTF8(message_str))
328-
message_str = ToHex(message_str);
329-
330325
// Convert to checksum address
331326
auto checksum_address = EthAddress::FromHex(address);
332327
GetAllowedAccounts(base::BindOnce(
333328
&BraveWalletProviderImpl::ContinueSignMessage, weak_factory_.GetWeakPtr(),
334-
checksum_address.ToChecksumAddress(), message_str,
335-
std::move(message_bytes), std::move(callback), false));
336-
}
337-
338-
void BraveWalletProviderImpl::SignTypedMessage(
339-
const std::string& address,
340-
const std::string& message,
341-
const std::string& message_to_sign,
342-
base::Value domain,
343-
SignTypedMessageCallback callback) {
344-
std::vector<uint8_t> eip712_hash;
345-
if (!EthAddress::IsValidAddress(address) ||
346-
!base::HexStringToBytes(message_to_sign, &eip712_hash) ||
347-
eip712_hash.size() != 32 || !domain.is_dict()) {
348-
std::move(callback).Run(
349-
"", static_cast<int>(ProviderErrors::kInvalidParams),
350-
l10n_util::GetStringUTF8(IDS_WALLET_INVALID_PARAMETERS));
351-
return;
352-
}
353-
354-
auto chain_id = domain.FindDoubleKey("chainId");
355-
if (chain_id) {
356-
const std::string chain_id_hex =
357-
Uint256ValueToHex((uint256_t)(uint64_t)*chain_id);
358-
if (chain_id_hex != rpc_controller_->GetChainId()) {
359-
std::move(callback).Run(
360-
"", static_cast<int>(ProviderErrors::kInternalError),
361-
l10n_util::GetStringFUTF8(
362-
IDS_BRAVE_WALLET_SIGN_TYPED_MESSAGE_CHAIN_ID_MISMATCH,
363-
base::ASCIIToUTF16(chain_id_hex)));
364-
return;
365-
}
366-
}
367-
368-
// Convert to checksum address
369-
auto checksum_address = EthAddress::FromHex(address);
370-
GetAllowedAccounts(base::BindOnce(
371-
&BraveWalletProviderImpl::ContinueSignMessage, weak_factory_.GetWeakPtr(),
372-
checksum_address.ToChecksumAddress(), message, std::move(eip712_hash),
373-
std::move(callback), true));
329+
checksum_address.ToChecksumAddress(), std::move(message_bytes),
330+
std::move(callback)));
374331
}
375332

376333
void BraveWalletProviderImpl::ContinueSignMessage(
377334
const std::string& address,
378-
const std::string& message,
379-
std::vector<uint8_t>&& message_to_sign,
335+
std::vector<uint8_t>&& message,
380336
SignMessageCallback callback,
381-
bool is_eip712,
382337
bool success,
383338
const std::vector<std::string>& allowed_accounts) {
384339
if (!CheckAccountAllowed(address, allowed_accounts)) {
@@ -389,21 +344,25 @@ void BraveWalletProviderImpl::ContinueSignMessage(
389344
return;
390345
}
391346

347+
std::string message_str(message.begin(), message.end());
348+
if (!base::IsStringUTF8(message_str))
349+
message_str = ToHex(message_str);
350+
392351
auto request =
393-
mojom::SignMessageRequest::New(sign_message_id_++, address, message);
352+
mojom::SignMessageRequest::New(sign_message_id_++, address, message_str);
394353
if (keyring_controller_->IsHardwareAccount(address)) {
395354
brave_wallet_service_->AddSignMessageRequest(
396355
std::move(request),
397356
base::BindOnce(
398357
&BraveWalletProviderImpl::OnHardwareSignMessageRequestProcessed,
399358
weak_factory_.GetWeakPtr(), std::move(callback), address,
400-
std::move(message_to_sign), is_eip712));
359+
std::move(message)));
401360
} else {
402361
brave_wallet_service_->AddSignMessageRequest(
403362
std::move(request),
404363
base::BindOnce(&BraveWalletProviderImpl::OnSignMessageRequestProcessed,
405364
weak_factory_.GetWeakPtr(), std::move(callback), address,
406-
std::move(message_to_sign), is_eip712));
365+
std::move(message)));
407366
}
408367
delegate_->ShowBubble();
409368
}
@@ -412,7 +371,6 @@ void BraveWalletProviderImpl::OnSignMessageRequestProcessed(
412371
SignMessageCallback callback,
413372
const std::string& address,
414373
std::vector<uint8_t>&& message,
415-
bool is_eip712,
416374
bool approved,
417375
const std::string& signature,
418376
const std::string& error) {
@@ -423,8 +381,8 @@ void BraveWalletProviderImpl::OnSignMessageRequestProcessed(
423381
return;
424382
}
425383

426-
auto signature_with_err = keyring_controller_->SignMessageByDefaultKeyring(
427-
address, message, is_eip712);
384+
auto signature_with_err =
385+
keyring_controller_->SignMessageByDefaultKeyring(address, message);
428386
if (!signature_with_err.signature)
429387
std::move(callback).Run("",
430388
static_cast<int>(ProviderErrors::kInternalError),
@@ -437,7 +395,6 @@ void BraveWalletProviderImpl::OnHardwareSignMessageRequestProcessed(
437395
SignMessageCallback callback,
438396
const std::string& address,
439397
std::vector<uint8_t>&& message,
440-
bool is_eip712,
441398
bool approved,
442399
const std::string& signature,
443400
const std::string& error) {

‎components/brave_wallet/browser/brave_wallet_provider_impl.h

+1-10
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,6 @@ class BraveWalletProviderImpl final
7373
void SignMessage(const std::string& address,
7474
const std::string& message,
7575
SignMessageCallback callback) override;
76-
void SignTypedMessage(const std::string& address,
77-
const std::string& message,
78-
const std::string& message_to_sign,
79-
base::Value domain,
80-
SignTypedMessageCallback callback) override;
8176
void OnGetAllowedAccounts(GetAllowedAccountsCallback callback,
8277
bool success,
8378
const std::vector<std::string>& accounts);
@@ -140,10 +135,8 @@ class BraveWalletProviderImpl final
140135
bool success,
141136
const std::vector<std::string>& allowed_accounts);
142137
void ContinueSignMessage(const std::string& address,
143-
const std::string& message,
144-
std::vector<uint8_t>&& message_to_sign,
138+
std::vector<uint8_t>&& message,
145139
SignMessageCallback callback,
146-
bool is_eip712,
147140
bool success,
148141
const std::vector<std::string>& allowed_accounts);
149142
bool CheckAccountAllowed(const std::string& account,
@@ -168,14 +161,12 @@ class BraveWalletProviderImpl final
168161
void OnSignMessageRequestProcessed(SignMessageCallback callback,
169162
const std::string& address,
170163
std::vector<uint8_t>&& message,
171-
bool is_eip712,
172164
bool approved,
173165
const std::string& signature,
174166
const std::string& error);
175167
void OnHardwareSignMessageRequestProcessed(SignMessageCallback callback,
176168
const std::string& address,
177169
std::vector<uint8_t>&& message,
178-
bool is_eip712,
179170
bool approved,
180171
const std::string& signature,
181172
const std::string& error);

‎components/brave_wallet/browser/hd_keyring.cc

+7-17
Original file line numberDiff line numberDiff line change
@@ -127,27 +127,17 @@ void HDKeyring::SignTransaction(const std::string& address,
127127

128128
std::vector<uint8_t> HDKeyring::SignMessage(const std::string& address,
129129
const std::vector<uint8_t>& message,
130-
uint256_t chain_id,
131-
bool is_eip712) {
130+
uint256_t chain_id) {
132131
HDKey* hd_key = GetHDKeyFromAddress(address);
133132
if (!hd_key)
134133
return std::vector<uint8_t>();
135134

136-
std::vector<uint8_t> hash;
137-
if (!is_eip712) {
138-
std::string prefix("\x19");
139-
prefix += std::string("Ethereum Signed Message:\n" +
140-
base::NumberToString(message.size()));
141-
std::vector<uint8_t> hash_input(prefix.begin(), prefix.end());
142-
hash_input.insert(hash_input.end(), message.begin(), message.end());
143-
hash = KeccakHash(hash_input);
144-
} else {
145-
// eip712 hash is Keccak
146-
if (message.size() != 32)
147-
return std::vector<uint8_t>();
148-
149-
hash = message;
150-
}
135+
std::string prefix("\x19");
136+
prefix += std::string("Ethereum Signed Message:\n" +
137+
base::NumberToString(message.size()));
138+
std::vector<uint8_t> hash_input(prefix.begin(), prefix.end());
139+
hash_input.insert(hash_input.end(), message.begin(), message.end());
140+
const std::vector<uint8_t> hash = KeccakHash(hash_input);
151141

152142
int recid;
153143
std::vector<uint8_t> signature = hd_key->Sign(hash, &recid);

‎components/brave_wallet/browser/hd_keyring.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,7 @@ class HDKeyring {
5656
// eth_sign
5757
virtual std::vector<uint8_t> SignMessage(const std::string& address,
5858
const std::vector<uint8_t>& message,
59-
uint256_t chain_id,
60-
bool is_eip712);
59+
uint256_t chain_id);
6160

6261
HDKey* GetHDKeyFromAddress(const std::string& address);
6362

0 commit comments

Comments
 (0)