Skip to content

Commit

Permalink
Incorrect block explorer launched with the correct transaction hash (#…
Browse files Browse the repository at this point in the history
…17181)

* Implemented Migration + chain_id for the TransactionInfo

Signed-off-by: Vadym Struts <vstruts@brave.com>
  • Loading branch information
vadimstruts authored Mar 2, 2023
1 parent 6fff471 commit 24619d2
Show file tree
Hide file tree
Showing 25 changed files with 280 additions and 5 deletions.
60 changes: 60 additions & 0 deletions browser/brave_wallet/brave_wallet_prefs_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@
#include <memory>
#include <utility>

#include "base/strings/string_util.h"
#include "base/test/scoped_feature_list.h"
#include "brave/components/brave_wallet/browser/brave_wallet_constants.h"
#include "brave/components/brave_wallet/browser/brave_wallet_utils.h"
#include "brave/components/brave_wallet/browser/pref_names.h"
#include "brave/components/brave_wallet/common/features.h"
#include "brave/components/brave_wallet/common/test_utils.h"
#include "chrome/browser/prefs/browser_prefs.h"
#include "chrome/test/base/testing_profile.h"
#include "components/prefs/pref_service.h"
Expand Down Expand Up @@ -156,6 +158,7 @@ TEST_F(BraveWalletPrefsUnitTest,

TEST_F(BraveWalletPrefsUnitTest,
MigrateObsoleteProfilePrefsBraveWalletEthereumTransactionsCoinType) {
GetPrefs()->SetBoolean(kBraveWalletTransactionsChainIdMigrated, true);
// Migration when kBraveWalletTransactions is default value (empty dict).
ASSERT_FALSE(
GetPrefs()->GetBoolean(kBraveWalletEthereumTransactionsCoinTypeMigrated));
Expand All @@ -169,6 +172,7 @@ TEST_F(BraveWalletPrefsUnitTest,
// Migration with existing transactions.
GetPrefs()->ClearPref(kBraveWalletEthereumTransactionsCoinTypeMigrated);
GetPrefs()->ClearPref(kBraveWalletTransactions);
GetPrefs()->SetBoolean(kBraveWalletTransactionsChainIdMigrated, true);
ASSERT_FALSE(
GetPrefs()->GetBoolean(kBraveWalletEthereumTransactionsCoinTypeMigrated));
base::Value::Dict tx1;
Expand Down Expand Up @@ -274,4 +278,60 @@ TEST_F(BraveWalletPrefsUnitTest, MigrateShowTestNetworksToggle) {
EXPECT_FALSE(GetPrefs()->HasPrefPath(kShowWalletTestNetworksDeprecated));
}

TEST_F(BraveWalletPrefsUnitTest, MigrateAddChainIdToTransactionInfo) {
GetPrefs()->SetBoolean(kBraveWalletEthereumTransactionsCoinTypeMigrated,
true);
EXPECT_FALSE(
GetPrefs()->HasPrefPath(kBraveWalletTransactionsChainIdMigrated));
EXPECT_FALSE(GetPrefs()->GetBoolean(kBraveWalletTransactionsChainIdMigrated));

base::Value::Dict txs;
const char ethTxId[] = "b1e8dda1";
const auto ethPath = base::JoinString({"mainnet", ethTxId}, ".");
base::Value::Dict tx1;
tx1.SetByDottedPath(base::JoinString({ethPath, "id"}, "."), ethTxId);

const char solTxId[] = "887e878f";
const auto solPath = base::JoinString({"devnet", solTxId}, ".");
base::Value::Dict tx2;
tx2.SetByDottedPath(base::JoinString({solPath, "id"}, "."), solTxId);

const char filTxId[] = "197ea1e5";
const auto filPath = base::JoinString({"testnet", filTxId}, ".");
base::Value::Dict tx3;
tx3.SetByDottedPath(base::JoinString({filPath, "id"}, "."), filTxId);

{
ScopedDictPrefUpdate update(GetPrefs(), kBraveWalletTransactions);
update->SetByDottedPath("ethereum", std::move(tx1));
update->SetByDottedPath("solana", std::move(tx2));
update->SetByDottedPath("filecoin", std::move(tx3));
}
MigrateObsoleteProfilePrefs(GetPrefs());

EXPECT_TRUE(GetPrefs()->HasPrefPath(kBraveWalletTransactionsChainIdMigrated));
EXPECT_TRUE(GetPrefs()->GetBoolean(kBraveWalletTransactionsChainIdMigrated));

const base::Value::Dict* transactions =
&GetPrefs()->GetDict(kBraveWalletTransactions);

const std::string* ethChainId = transactions->FindStringByDottedPath(
base::JoinString({"ethereum", ethPath, "chain_id"}, "."));

EXPECT_TRUE(ethChainId != nullptr);
EXPECT_EQ(*ethChainId, "0x1");

const std::string* solChainId = transactions->FindStringByDottedPath(
base::JoinString({"solana", solPath, "chain_id"}, "."));

EXPECT_FALSE(solChainId == nullptr);
EXPECT_EQ(*solChainId, "0x67");

const std::string* filChainId = transactions->FindStringByDottedPath(
base::JoinString({"filecoin", filPath, "chain_id"}, "."));

EXPECT_FALSE(filChainId == nullptr);
EXPECT_EQ(*filChainId, "t");
}

} // namespace brave_wallet
8 changes: 7 additions & 1 deletion components/brave_wallet/browser/brave_wallet_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "brave/components/brave_wallet/browser/brave_wallet_utils.h"
#include "brave/components/brave_wallet/browser/json_rpc_service.h"
#include "brave/components/brave_wallet/browser/pref_names.h"
#include "brave/components/brave_wallet/browser/tx_state_manager.h"
#include "brave/components/brave_wallet/common/brave_wallet.mojom.h"
#include "brave/components/brave_wallet/common/pref_names.h"
#include "brave/components/p3a_utils/feature_usage.h"
Expand Down Expand Up @@ -170,6 +171,9 @@ void RegisterProfilePrefsForMigration(

// Added 12/2022
registry->RegisterBooleanPref(kShowWalletTestNetworksDeprecated, false);

// Added 02/2023
registry->RegisterBooleanPref(kBraveWalletTransactionsChainIdMigrated, false);
}

void ClearJsonRpcServiceProfilePrefs(PrefService* prefs) {
Expand Down Expand Up @@ -251,12 +255,14 @@ void MigrateObsoleteProfilePrefs(PrefService* prefs) {
}
prefs->SetBoolean(kBraveWalletEthereumTransactionsCoinTypeMigrated, true);
}

// Added 10/2022
JsonRpcService::MigrateDeprecatedEthereumTestnets(prefs);

// Added 12/2022
JsonRpcService::MigrateShowTestNetworksToggle(prefs);

// Added 02/2023
TxStateManager::MigrateAddChainIdToTransactionInfo(prefs);
}

} // namespace brave_wallet
15 changes: 15 additions & 0 deletions components/brave_wallet/browser/brave_wallet_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1189,6 +1189,21 @@ absl::optional<std::string> GetChainId(PrefService* prefs,
return absl::nullopt;
}

absl::optional<std::string> GetChainIdByNetworkId(
PrefService* prefs,
const mojom::CoinType& coin,
const std::string& network_id) {
if (network_id.empty()) {
return absl::nullopt;
}
for (const auto& network : GetAllChains(prefs, coin)) {
if (network_id == GetNetworkId(prefs, coin, network->chain_id)) {
return network->chain_id;
}
}
return absl::nullopt;
}

mojom::DefaultWallet GetDefaultEthereumWallet(PrefService* prefs) {
return static_cast<brave_wallet::mojom::DefaultWallet>(
prefs->GetInteger(kDefaultEthereumWallet));
Expand Down
6 changes: 6 additions & 0 deletions components/brave_wallet/browser/brave_wallet_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,12 @@ absl::optional<std::string> GetChainId(PrefService* prefs,
const mojom::CoinType& coin,
const std::string& network_id);

// Resolves chain_id from network_id (including custom networks).
absl::optional<std::string> GetChainIdByNetworkId(
PrefService* prefs,
const mojom::CoinType& coin,
const std::string& network_id);

// Returns a string used for web3_clientVersion in the form of
// BraveWallet/v[chromium-version]. Note that we expose only the Chromium
// version and not the Brave version because that way no extra entropy
Expand Down
54 changes: 54 additions & 0 deletions components/brave_wallet/browser/brave_wallet_utils_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1187,4 +1187,58 @@ TEST(BraveWalletUtilsUnitTest, GetSnsRpcUrl) {
EXPECT_EQ(GURL("https://mainnet-beta-solana.brave.com/rpc"), GetSnsRpcUrl());
}

TEST(BraveWalletUtilsUnitTest, GetChainIdByNetworkId) {
TestingPrefServiceSimple prefs;
prefs.registry()->RegisterDictionaryPref(kBraveWalletCustomNetworks);
prefs.registry()->RegisterBooleanPref(kSupportEip1559OnLocalhostChain, false);

for (auto coin :
{mojom::CoinType::ETH, mojom::CoinType::FIL, mojom::CoinType::SOL}) {
ASSERT_TRUE(GetAllCustomChains(&prefs, coin).empty());
std::vector<base::Value::Dict> values;
mojom::NetworkInfo chain1 = GetTestNetworkInfo1();
chain1.coin = coin;
values.push_back(NetworkInfoToValue(chain1));
mojom::NetworkInfo chain2 = GetTestNetworkInfo2();
chain2.coin = coin;
if (coin != mojom::CoinType::ETH) {
chain2.is_eip1559 = false;
}
values.push_back(NetworkInfoToValue(chain2));
UpdateCustomNetworks(&prefs, std::move(values), coin);
}

auto getChainIdByNetworkIdCheck = [&](const mojom::CoinType& coin_type) {
for (const auto& chain : GetAllChains(&prefs, coin_type)) {
std::string nid;
if (chain->coin == mojom::CoinType::ETH) {
nid = GetKnownEthNetworkId(chain->chain_id);
}
if (chain->coin == mojom::CoinType::SOL) {
nid = GetKnownSolNetworkId(chain->chain_id);
}
if (chain->coin == mojom::CoinType::FIL) {
nid = GetKnownFilNetworkId(chain->chain_id);
}
if (nid.empty()) {
nid = chain->chain_id;
// GetNetworkId supports only ETH for custom networks atm.
if (chain->coin != mojom::CoinType::ETH) {
ASSERT_FALSE(GetChainIdByNetworkId(&prefs, coin_type, nid));
continue;
}
}
auto chainId = GetChainIdByNetworkId(&prefs, coin_type, nid);
ASSERT_TRUE(chainId.has_value());
EXPECT_EQ(chain->chain_id, chainId.value());
}
ASSERT_FALSE(GetChainIdByNetworkId(&prefs, coin_type, ""));
};

for (auto coin :
{mojom::CoinType::ETH, mojom::CoinType::FIL, mojom::CoinType::SOL}) {
getChainIdByNetworkIdCheck(coin);
}
}

} // namespace brave_wallet
2 changes: 2 additions & 0 deletions components/brave_wallet/browser/eth_tx_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "base/strings/string_util.h"
#include "brave/components/brave_wallet/browser/brave_wallet_constants.h"
#include "brave/components/brave_wallet/browser/brave_wallet_prefs.h"
#include "brave/components/brave_wallet/browser/brave_wallet_utils.h"
#include "brave/components/brave_wallet/browser/eip1559_transaction.h"
#include "brave/components/brave_wallet/browser/eth_data_builder.h"
#include "brave/components/brave_wallet/browser/eth_data_parser.h"
Expand Down Expand Up @@ -284,6 +285,7 @@ void EthTxManager::ContinueAddUnapprovedTransaction(
meta.set_created_time(base::Time::Now());
meta.set_status(mojom::TransactionStatus::Unapproved);
meta.set_sign_only(sign_only);
meta.set_chain_id(GetCurrentChainId(prefs_, mojom::CoinType::ETH));
tx_state_manager_->AddOrUpdateTx(meta);
std::move(callback).Run(true, meta.id(), "");
}
Expand Down
3 changes: 2 additions & 1 deletion components/brave_wallet/browser/eth_tx_meta.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ mojom::TransactionInfoPtr EthTxMeta::ToTransactionInfo() const {
base::Milliseconds(created_time_.ToJavaTime()),
base::Milliseconds(submitted_time_.ToJavaTime()),
base::Milliseconds(confirmed_time_.ToJavaTime()),
origin_.has_value() ? MakeOriginInfo(*origin_) : nullptr, group_id_);
origin_.has_value() ? MakeOriginInfo(*origin_) : nullptr, group_id_,
chain_id_);
}

} // namespace brave_wallet
3 changes: 3 additions & 0 deletions components/brave_wallet/browser/fil_tx_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <vector>

#include "base/notreached.h"
#include "brave/components/brave_wallet/browser/brave_wallet_utils.h"
#include "brave/components/brave_wallet/browser/fil_block_tracker.h"
#include "brave/components/brave_wallet/browser/fil_nonce_tracker.h"
#include "brave/components/brave_wallet/browser/fil_transaction.h"
Expand Down Expand Up @@ -91,6 +92,8 @@ void FilTxManager::ContinueAddUnapprovedTransaction(
meta.set_group_id(group_id);
meta.set_created_time(base::Time::Now());
meta.set_status(mojom::TransactionStatus::Unapproved);
meta.set_chain_id(GetCurrentChainId(prefs_, mojom::CoinType::FIL));

tx_state_manager_->AddOrUpdateTx(meta);
std::move(callback).Run(true, meta.id(), "");
}
Expand Down
22 changes: 22 additions & 0 deletions components/brave_wallet/browser/fil_tx_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,8 @@ TEST_F(FilTxManagerUnitTest, SubmitTransactions) {

auto tx_meta1 = fil_tx_manager()->GetTxForTesting(meta_id1);
EXPECT_TRUE(tx_meta1);
EXPECT_EQ(tx_meta1->chain_id(),
GetCurrentChainId(prefs(), mojom::CoinType::FIL));

EXPECT_EQ(tx_meta1->tx()->gas_fee_cap(), "100820");
EXPECT_EQ(tx_meta1->tx()->gas_limit(), 598585);
Expand All @@ -273,6 +275,8 @@ TEST_F(FilTxManagerUnitTest, SubmitTransactions) {
&meta_id2);
auto tx_meta2 = fil_tx_manager()->GetTxForTesting(meta_id2);
ASSERT_TRUE(tx_meta2);
EXPECT_EQ(tx_meta2->chain_id(),
GetCurrentChainId(prefs(), mojom::CoinType::FIL));
EXPECT_EQ(tx_meta2->from(), from_account);
EXPECT_EQ(tx_meta2->status(), mojom::TransactionStatus::Unapproved);

Expand Down Expand Up @@ -380,6 +384,8 @@ TEST_F(FilTxManagerUnitTest, SubmitTransactionConfirmed) {

auto tx_meta1 = fil_tx_manager()->GetTxForTesting(meta_id1);
EXPECT_TRUE(tx_meta1);
EXPECT_EQ(tx_meta1->chain_id(),
GetCurrentChainId(prefs(), mojom::CoinType::FIL));

EXPECT_EQ(tx_meta1->tx()->gas_fee_cap(), "100820");
EXPECT_EQ(tx_meta1->tx()->gas_limit(), 598585);
Expand Down Expand Up @@ -436,6 +442,8 @@ TEST_F(FilTxManagerUnitTest, WalletOrigin) {

auto tx_meta = fil_tx_manager()->GetTxForTesting(meta_id);
ASSERT_TRUE(tx_meta);
EXPECT_EQ(tx_meta->chain_id(),
GetCurrentChainId(prefs(), mojom::CoinType::FIL));

EXPECT_EQ(tx_meta->origin(), url::Origin::Create(GURL("chrome://wallet")));
}
Expand All @@ -456,6 +464,8 @@ TEST_F(FilTxManagerUnitTest, SomeSiteOrigin) {
ASSERT_TRUE(tx_meta);
EXPECT_EQ(tx_meta->origin(),
url::Origin::Create(GURL("https://some.site.com")));
EXPECT_EQ(tx_meta->chain_id(),
GetCurrentChainId(prefs(), mojom::CoinType::FIL));
}

TEST_F(FilTxManagerUnitTest, AddUnapprovedTransactionWithGroupId) {
Expand All @@ -473,13 +483,17 @@ TEST_F(FilTxManagerUnitTest, AddUnapprovedTransactionWithGroupId) {
auto tx_meta = fil_tx_manager()->GetTxForTesting(meta_id);
ASSERT_TRUE(tx_meta);
EXPECT_EQ(tx_meta->group_id(), "mockGroupId");
EXPECT_EQ(tx_meta->chain_id(),
GetCurrentChainId(prefs(), mojom::CoinType::FIL));

// Transaction with empty group_id
AddUnapprovedTransaction(tx_data.Clone(), from_account, absl::nullopt,
&meta_id);
tx_meta = fil_tx_manager()->GetTxForTesting(meta_id);
ASSERT_TRUE(tx_meta);
EXPECT_EQ(tx_meta->group_id(), absl::nullopt);
EXPECT_EQ(tx_meta->chain_id(),
GetCurrentChainId(prefs(), mojom::CoinType::FIL));
}

TEST_F(FilTxManagerUnitTest, GetTransactionMessageToSign) {
Expand All @@ -495,6 +509,8 @@ TEST_F(FilTxManagerUnitTest, GetTransactionMessageToSign) {
&meta_id);
auto tx_meta = fil_tx_manager()->GetTxForTesting(meta_id);
ASSERT_TRUE(tx_meta);
EXPECT_EQ(tx_meta->chain_id(),
GetCurrentChainId(prefs(), mojom::CoinType::FIL));
EXPECT_EQ(tx_meta->from(), from_account);
EXPECT_EQ(tx_meta->status(), mojom::TransactionStatus::Unapproved);
GetTransactionMessageToSign(meta_id, R"(
Expand Down Expand Up @@ -526,6 +542,8 @@ TEST_F(FilTxManagerUnitTest, GetTransactionMessageToSign) {
&meta_id);
auto tx_meta = fil_tx_manager()->GetTxForTesting(meta_id);
ASSERT_TRUE(tx_meta);
EXPECT_EQ(tx_meta->chain_id(),
GetCurrentChainId(prefs(), mojom::CoinType::FIL));
EXPECT_EQ(tx_meta->from(), from_account);
EXPECT_EQ(tx_meta->status(), mojom::TransactionStatus::Unapproved);
GetTransactionMessageToSign(meta_id, R"(
Expand Down Expand Up @@ -560,6 +578,8 @@ TEST_F(FilTxManagerUnitTest, ProcessHardwareSignature) {
&meta_id);
auto tx_meta = fil_tx_manager()->GetTxForTesting(meta_id);
ASSERT_TRUE(tx_meta);
EXPECT_EQ(tx_meta->chain_id(),
GetCurrentChainId(prefs(), mojom::CoinType::FIL));
EXPECT_EQ(tx_meta->from(), from_account);
EXPECT_EQ(tx_meta->status(), mojom::TransactionStatus::Unapproved);
auto signed_message =
Expand Down Expand Up @@ -624,6 +644,8 @@ TEST_F(FilTxManagerUnitTest, ProcessHardwareSignatureError) {
&meta_id);
auto tx_meta = fil_tx_manager()->GetTxForTesting(meta_id);
ASSERT_TRUE(tx_meta);
EXPECT_EQ(tx_meta->chain_id(),
GetCurrentChainId(prefs(), mojom::CoinType::FIL));
EXPECT_EQ(tx_meta->from(), from_account);
EXPECT_EQ(tx_meta->status(), mojom::TransactionStatus::Unapproved);
auto signed_message =
Expand Down
3 changes: 2 additions & 1 deletion components/brave_wallet/browser/fil_tx_meta.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ mojom::TransactionInfoPtr FilTxMeta::ToTransactionInfo() const {
base::Milliseconds(created_time_.ToJavaTime()),
base::Milliseconds(submitted_time_.ToJavaTime()),
base::Milliseconds(confirmed_time_.ToJavaTime()),
origin_.has_value() ? MakeOriginInfo(*origin_) : nullptr, group_id_);
origin_.has_value() ? MakeOriginInfo(*origin_) : nullptr, group_id_,
chain_id_);
}

} // namespace brave_wallet
2 changes: 2 additions & 0 deletions components/brave_wallet/browser/fil_tx_meta_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ TEST(FilTxMeta, ToTransactionInfo) {
"1", "2", "3", "4", "5", "t1h4n7rphclbmwyjcp6jrdiwlfcuwbroxy3jvg33q",
"t1h5tg3bhp5r56uzgjae2373znti6ygq4agkx4hzq", "6")));
FilTxMeta meta(std::move(tx));
meta.set_chain_id("0x66");
meta.set_from("t1h5tg3bhp5r56uzgjae2373znti6ygq4agkx4hzq");
base::Time::Exploded x{1981, 3, 0, 1, 2};
base::Time confirmed_time = meta.confirmed_time();
Expand All @@ -28,6 +29,7 @@ TEST(FilTxMeta, ToTransactionInfo) {

mojom::TransactionInfoPtr ti = meta.ToTransactionInfo();
EXPECT_EQ(ti->id, meta.id());
EXPECT_EQ(ti->chain_id, meta.chain_id());
EXPECT_EQ(ti->from_address, meta.from());
EXPECT_EQ(ti->tx_status, meta.status());
EXPECT_TRUE(ti->tx_data_union->is_fil_tx_data());
Expand Down
2 changes: 2 additions & 0 deletions components/brave_wallet/browser/pref_names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ const char kBraveWalletLastTransactionSentTimeDict[] =
"brave.wallet.last_transaction_sent_time_dict";
const char kBraveWalletLastDiscoveredAssetsAt[] =
"brave.wallet.last_discovered_assets_at";
const char kBraveWalletTransactionsChainIdMigrated[] =
"brave.wallet.transactions.chain_id_migrated";

// DEPRECATED
const char kShowWalletTestNetworksDeprecated[] =
Expand Down
Loading

0 comments on commit 24619d2

Please sign in to comment.