Skip to content

Commit

Permalink
Launch onboarding at most once per Solana provider (uplift to 1.49.x) (…
Browse files Browse the repository at this point in the history 
…#17787)

Merge pull request #17241 from brave/onboarding-max-1

Launch onboarding at most once per Solana provider
  • Loading branch information
@darkdh
darkdh authored Mar 28, 2023
1 parent a88eaa9 commit e7584b1
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 27 deletions.
41 changes: 33 additions & 8 deletions browser/brave_wallet/solana_provider_impl_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ class TestEventsListener : public mojom::SolanaEventsListener {

void AccountChangedEvent(
const absl::optional<std::string>& account) override {
if (account.has_value())
if (account.has_value()) {
account_ = *account;
}
account_changed_fired_ = true;
}

Expand Down Expand Up @@ -155,8 +156,9 @@ class SolanaProviderImplUnitTest : public testing::Test {
brave_wallet_service_->GetPendingSignMessageRequests(
base::BindLambdaForTesting(
[&](std::vector<mojom::SignMessageRequestPtr> requests) {
for (const auto& request : requests)
for (const auto& request : requests) {
requests_out.push_back(request.Clone());
}
run_loop.Quit();
}));
run_loop.Run();
Expand Down Expand Up @@ -264,10 +266,12 @@ class SolanaProviderImplUnitTest : public testing::Test {
base::BindLambdaForTesting([&](mojom::SolanaProviderError error,
const std::string& error_message,
const std::string& public_key) {
if (error_out)
if (error_out) {
*error_out = error;
if (error_message_out)
}
if (error_message_out) {
*error_message_out = error_message;
}
account = public_key;
run_loop.Quit();
}));
Expand All @@ -291,13 +295,16 @@ class SolanaProviderImplUnitTest : public testing::Test {
base::BindLambdaForTesting([&](mojom::SolanaProviderError error,
const std::string& error_message,
base::Value::Dict result) {
if (error_out)
if (error_out) {
*error_out = error;
if (error_message_out)
}
if (error_message_out) {
*error_message_out = error_message;
}
const std::string* signature = result.FindString("signature");
if (signature)
if (signature) {
signature_out = *signature;
}
run_loop.Quit();
}));

Expand Down Expand Up @@ -411,8 +418,9 @@ class SolanaProviderImplUnitTest : public testing::Test {
const std::string& expected_error_message) {
base::Value::Dict result_out;
auto value = base::JSONReader::Read(json);
if (!value)
if (!value) {
return result_out;
}
base::RunLoop run_loop;
provider_->Request(
value->GetDict().Clone(),
Expand Down Expand Up @@ -597,16 +605,33 @@ TEST_F(SolanaProviderImplUnitTest, ConnectWithNoSolanaAccount) {
EXPECT_EQ(error, mojom::SolanaProviderError::kInternalError);
EXPECT_FALSE(IsConnected());
EXPECT_TRUE(account_creation_callback_called);
EXPECT_TRUE(provider_->account_creation_shown_);

provider_->account_creation_shown_ = false;
account_creation_callback_called = false;
SetCallbackForAccountCreationForTesting(base::BindLambdaForTesting(
[&]() { account_creation_callback_called = true; }));
// No solana account
CreateWallet();
keyring_service_->RemoveSelectedAccountForCoin(mojom::CoinType::SOL,
mojom::kSolanaKeyringId);
account = Connect(absl::nullopt, &error, &error_message);
EXPECT_TRUE(account.empty());
EXPECT_EQ(error, mojom::SolanaProviderError::kInternalError);
EXPECT_FALSE(IsConnected());
EXPECT_TRUE(account_creation_callback_called);
EXPECT_TRUE(provider_->account_creation_shown_);

// It should be shown at most once.
account_creation_callback_called = false;
SetCallbackForAccountCreationForTesting(base::BindLambdaForTesting(
[&]() { account_creation_callback_called = true; }));
account = Connect(absl::nullopt, &error, &error_message);
EXPECT_TRUE(account.empty());
EXPECT_EQ(error, mojom::SolanaProviderError::kInternalError);
EXPECT_FALSE(IsConnected());
EXPECT_FALSE(account_creation_callback_called);
EXPECT_TRUE(provider_->account_creation_shown_);
}

TEST_F(SolanaProviderImplUnitTest, Disconnect) {
Expand Down
2 changes: 2 additions & 0 deletions components/brave_wallet/browser/keyring_service.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,8 @@ class KeyringService : public KeyedService, public mojom::KeyringService {
RestoreWalletTwice);
FRIEND_TEST_ALL_PREFIXES(AssetDiscoveryManagerUnitTest,
KeyringServiceObserver);
FRIEND_TEST_ALL_PREFIXES(SolanaProviderImplUnitTest,
ConnectWithNoSolanaAccount);

friend class EthereumProviderImplUnitTest;
friend class SolanaProviderImplUnitTest;
Expand Down
55 changes: 36 additions & 19 deletions components/brave_wallet/browser/solana_provider_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,10 @@ void SolanaProviderImpl::Connect(absl::optional<base::Value::Dict> arg,
if (!account) {
// Prompt users to create a Solana account. If wallet is not setup, users
// will be lead to onboarding first.
delegate_->ShowAccountCreation(mojom::CoinType::SOL);
if (!account_creation_shown_) {
delegate_->ShowAccountCreation(mojom::CoinType::SOL);
account_creation_shown_ = true;
}
std::move(callback).Run(mojom::SolanaProviderError::kInternalError,
l10n_util::GetStringUTF8(IDS_WALLET_INTERNAL_ERROR),
"");
Expand Down Expand Up @@ -119,18 +122,20 @@ void SolanaProviderImpl::Disconnect() {
DCHECK(keyring_service_);
absl::optional<std::string> account =
keyring_service_->GetSelectedAccount(mojom::CoinType::SOL);
if (account)
if (account) {
delegate_->RemoveSolanaConnectedAccount(*account);
}
}

void SolanaProviderImpl::IsConnected(IsConnectedCallback callback) {
DCHECK(keyring_service_);
absl::optional<std::string> account =
keyring_service_->GetSelectedAccount(mojom::CoinType::SOL);
if (!account)
if (!account) {
std::move(callback).Run(false);
else
} else {
std::move(callback).Run(IsAccountConnected(*account));
}
}

void SolanaProviderImpl::GetPublicKey(GetPublicKeyCallback callback) {
Expand All @@ -140,10 +145,11 @@ void SolanaProviderImpl::GetPublicKey(GetPublicKeyCallback callback) {
std::move(callback).Run("");
return;
}
if (IsAccountConnected(*account))
if (IsAccountConnected(*account)) {
std::move(callback).Run(*account);
else
} else {
std::move(callback).Run("");
}
}

absl::optional<std::pair<SolanaMessage, std::vector<uint8_t>>>
Expand All @@ -157,8 +163,9 @@ SolanaProviderImpl::GetDeserializedMessage(
}

auto msg = SolanaMessage::Deserialize(message_bytes);
if (!msg)
if (!msg) {
return absl::nullopt;
}

// Note: We cannot check Base58Encode(msg->Serialize()) is equal to the
// original encoded message passed in because the order of accounts with the
Expand Down Expand Up @@ -420,12 +427,14 @@ void SolanaProviderImpl::OnTransactionStatusChanged(
auto tx_status = tx_info->tx_status;
if (tx_status != mojom::TransactionStatus::Submitted &&
tx_status != mojom::TransactionStatus::Rejected &&
tx_status != mojom::TransactionStatus::Error)
tx_status != mojom::TransactionStatus::Error) {
return;
}

std::string tx_meta_id = tx_info->id;
if (!sign_and_send_tx_callbacks_.contains(tx_meta_id))
if (!sign_and_send_tx_callbacks_.contains(tx_meta_id)) {
return;
}

auto callback = std::move(sign_and_send_tx_callbacks_[tx_meta_id]);
base::Value::Dict result;
Expand Down Expand Up @@ -476,10 +485,11 @@ void SolanaProviderImpl::SignMessage(
return;
}
std::string message;
if (display_encoding && *display_encoding == "hex")
if (display_encoding && *display_encoding == "hex") {
message = base::StrCat({"0x", base::HexEncode(blob_msg)});
else
} else {
message = std::string(blob_msg.begin(), blob_msg.end());
}
auto request = mojom::SignMessageRequest::New(
MakeOriginInfo(delegate_->GetOrigin()), -1, *account, "", message, false,
absl::nullopt, absl::nullopt, blob_msg, mojom::CoinType::SOL);
Expand Down Expand Up @@ -516,8 +526,9 @@ void SolanaProviderImpl::Request(base::Value::Dict arg,

if (*method == solana::kConnect) {
absl::optional<base::Value::Dict> option = absl::nullopt;
if (params)
if (params) {
option = std::move(*params);
}
Connect(std::move(option),
base::BindOnce(&SolanaProviderImpl::OnRequestConnect,
weak_factory_.GetWeakPtr(), std::move(callback)));
Expand Down Expand Up @@ -550,8 +561,9 @@ void SolanaProviderImpl::Request(base::Value::Dict arg,
}
base::Value::Dict* options_dict = params->FindDict(kOptions);
absl::optional<base::Value::Dict> options = absl::nullopt;
if (options_dict)
if (options_dict) {
options = std::move(*options_dict);
}
SignAndSendTransaction(
mojom::SolanaSignTransactionParam::New(
*message, std::vector<mojom::SignaturePubkeyPairPtr>()),
Expand All @@ -570,8 +582,9 @@ void SolanaProviderImpl::Request(base::Value::Dict arg,
for (const auto& message : *messages) {
auto param = mojom::SolanaSignTransactionParam::New();
const std::string* encoded_serialized_msg = message.GetIfString();
if (encoded_serialized_msg)
if (encoded_serialized_msg) {
param->encoded_serialized_msg = *encoded_serialized_msg;
}
sign_params.push_back(std::move(param));
}
SignAllTransactions(
Expand All @@ -589,8 +602,9 @@ void SolanaProviderImpl::Request(base::Value::Dict arg,
}
const std::string* display_str = params->FindString("display");
absl::optional<std::string> display = absl::nullopt;
if (display_str)
if (display_str) {
display = *display_str;
}
SignMessage(*message, std::move(display), std::move(callback));
} else {
std::move(callback).Run(
Expand Down Expand Up @@ -711,8 +725,9 @@ void SolanaProviderImpl::OnRequestConnect(RequestCallback callback,
const std::string& error_message,
const std::string& public_key) {
base::Value::Dict result;
if (error == mojom::SolanaProviderError::kSuccess)
if (error == mojom::SolanaProviderError::kSuccess) {
result.Set(kPublicKey, public_key);
}
std::move(callback).Run(error, error_message, std::move(result));
}

Expand Down Expand Up @@ -762,16 +777,18 @@ void SolanaProviderImpl::Unlocked() {
}

void SolanaProviderImpl::SelectedAccountChanged(mojom::CoinType coin) {
if (!events_listener_.is_bound() || coin != mojom::CoinType::SOL)
if (!events_listener_.is_bound() || coin != mojom::CoinType::SOL) {
return;
}

DCHECK(keyring_service_);
absl::optional<std::string> account =
keyring_service_->GetSelectedAccount(mojom::CoinType::SOL);
if (account && IsAccountConnected(*account))
if (account && IsAccountConnected(*account)) {
events_listener_->AccountChangedEvent(account);
else
} else {
events_listener_->AccountChangedEvent(absl::nullopt);
}
}

} // namespace brave_wallet
3 changes: 3 additions & 0 deletions components/brave_wallet/browser/solana_provider_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ class SolanaProviderImpl final : public mojom::SolanaProvider,

private:
FRIEND_TEST_ALL_PREFIXES(SolanaProviderImplUnitTest, GetDeserializedMessage);
FRIEND_TEST_ALL_PREFIXES(SolanaProviderImplUnitTest,
ConnectWithNoSolanaAccount);

bool IsAccountConnected(const std::string& account);
void ContinueConnect(bool is_eagerly_connect,
Expand Down Expand Up @@ -147,6 +149,7 @@ class SolanaProviderImpl final : public mojom::SolanaProvider,
ConnectCallback pending_connect_callback_;
absl::optional<base::Value::Dict> pending_connect_arg_;

bool account_creation_shown_ = false;
mojo::Remote<mojom::SolanaEventsListener> events_listener_;
raw_ptr<KeyringService> keyring_service_ = nullptr;
raw_ptr<BraveWalletService> brave_wallet_service_ = nullptr;
Expand Down

0 comments on commit e7584b1

Please sign in to comment.