From fa52670bd30c0eb96811babf3322dd190969f7bd Mon Sep 17 00:00:00 2001 From: oisupov Date: Tue, 7 Mar 2023 02:51:59 +0400 Subject: [PATCH 1/4] Allow pinning of gateway-like urls Resolves https://github.com/brave/brave-browser/issues/28169 --- .../browser/brave_wallet_pin_service.cc | 40 ++++- .../brave_wallet_pin_service_unittest.cc | 138 +++++++++++++++++- 2 files changed, 174 insertions(+), 4 deletions(-) diff --git a/components/brave_wallet/browser/brave_wallet_pin_service.cc b/components/brave_wallet/browser/brave_wallet_pin_service.cc index 46592fe80869..782619920c08 100644 --- a/components/brave_wallet/browser/brave_wallet_pin_service.cc +++ b/components/brave_wallet/browser/brave_wallet_pin_service.cc @@ -85,14 +85,37 @@ absl::optional StringToErrorCode( return absl::nullopt; } -absl::optional ExtractCID(const std::string& ipfs_url) { - GURL gurl = GURL(ipfs_url); +absl::optional ExtractIpfsUrl(const std::string& url) { + GURL gurl = GURL(url); + + if (!gurl.is_valid()) { + return absl::nullopt; + } + + if (gurl.SchemeIs(ipfs::kIPFSScheme)) { + return gurl.spec(); + } + + auto scope = ipfs::ExtractSourceFromGateway(gurl); + if (!scope || !scope->SchemeIs(ipfs::kIPFSScheme)) { + return absl::nullopt; + } + + return scope.value().spec(); +} + +absl::optional ExtractCID(const std::string& url) { + GURL gurl = GURL(ExtractIpfsUrl(url).value_or("")); if (!gurl.is_valid()) { return absl::nullopt; } if (!gurl.SchemeIs(ipfs::kIPFSScheme)) { + auto scope = ipfs::ExtractSourceFromGateway(gurl); + if (!scope || !scope->SchemeIs(ipfs::kIPFSScheme)) { + return absl::nullopt; + } return absl::nullopt; } @@ -617,8 +640,19 @@ void BraveWalletPinService::OnTokenMetaDataReceived( cids.push_back(metadata_cid.value()); cids.push_back(image_cid.value()); + auto ipfs_image_url = ExtractIpfsUrl(*image_url); + if (!ipfs_image_url) { + auto pin_error = mojom::PinError::New( + mojom::WalletPinServiceErrorCode::ERR_NON_IPFS_TOKEN_URL, + "Can't find proper image field"); + SetTokenStatus(service, token, + mojom::TokenPinStatusCode::STATUS_PINNING_FAILED, pin_error); + std::move(callback).Run(false, std::move(pin_error)); + return; + } + content_type_checker_->CheckContentTypeSupported( - *image_url, + ipfs_image_url.value(), base::BindOnce(&BraveWalletPinService::OnContentTypeChecked, weak_ptr_factory_.GetWeakPtr(), std::move(service), std::move(token), std::move(cids), std::move(callback))); diff --git a/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc b/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc index 96ec7cf02c0e..276f8a243da0 100644 --- a/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc +++ b/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc @@ -40,6 +40,10 @@ const char kMonkey3Path[] = "nft.nftstorage.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x2"; const char kMonkey4Path[] = "nft.local.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x3"; +const char kMonkey5Path[] = + "nft.local.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x4"; +const char kMonkey6Path[] = + "nft.local.60.0x1.0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d.0x5"; const char kMonkey1Url[] = "ipfs://QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq/2413"; @@ -49,6 +53,10 @@ const char kMonkey3Url[] = "ipfs://QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq/2777"; const char kMonkey4Url[] = "ipfs://QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq/2888"; +const char kMonkey5Url[] = + "https://ipfs.io/ipfs/QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq/2888"; +const char kMonkey6Url[] = + "https://google.com/QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq/2888"; const char kMonkey1[] = R"({"image":"ipfs://bafy1", @@ -86,6 +94,24 @@ const char kMonkey4[] = {"trait_type":"Eyes","value":"Closed"}, {"trait_type":"Clothes","value":"Toga"}, {"trait_type":"Hat","value":"Cowboy Hat"}]})"; +const char kMonkey5[] = + R"({"image":"https://ipfs.io/ipfs/bafy3", + "attributes":[ + {"trait_type":"Mouth","value":"Bored Cigarette"}, + {"trait_type":"Fur","value":"Zombie"}, + {"trait_type":"Background","value":"Purple"}, + {"trait_type":"Eyes","value":"Closed"}, + {"trait_type":"Clothes","value":"Toga"}, + {"trait_type":"Hat","value":"Cowboy Hat"}]})"; +const char kMonkey6[] = + R"({"image":"https://google.com/bafy3", + "attributes":[ + {"trait_type":"Mouth","value":"Bored Cigarette"}, + {"trait_type":"Fur","value":"Zombie"}, + {"trait_type":"Background","value":"Purple"}, + {"trait_type":"Eyes","value":"Closed"}, + {"trait_type":"Clothes","value":"Toga"}, + {"trait_type":"Hat","value":"Cowboy Hat"}]})"; base::Time g_overridden_now; std::unique_ptr OverrideWithTimeNow( @@ -299,6 +325,116 @@ TEST_F(BraveWalletPinServiceTest, AddPin) { } } +TEST_F(BraveWalletPinServiceTest, AddPin_GatewayUrl) { + { + ON_CALL(*GetContentTypeChecker(), CheckContentTypeSupported(_, _)) + .WillByDefault(::testing::Invoke( + [](const std::string& cid, + base::OnceCallback)> callback) { + std::move(callback).Run(true); + })); + } + // Right gateway + { + ON_CALL(*GetJsonRpcService(), GetERC721Metadata(_, _, _, _)) + .WillByDefault(::testing::Invoke( + [](const std::string& contract_address, const std::string& token_id, + const std::string& chain_id, + MockJsonRpcService::GetERC721MetadataCallback callback) { + EXPECT_EQ("0x1", chain_id); + EXPECT_EQ("0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d", + contract_address); + EXPECT_EQ("0x4", token_id); + std::move(callback).Run(kMonkey5Url, kMonkey5, + mojom::ProviderError::kSuccess, ""); + })); + ON_CALL(*GetIpfsLocalPinService(), AddPins(_, _, _)) + .WillByDefault(::testing::Invoke( + [](const std::string& prefix, const std::vector& cids, + ipfs::AddPinCallback callback) { + EXPECT_EQ(kMonkey5Path, prefix); + EXPECT_EQ("QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq", + cids.at(0)); + EXPECT_EQ("bafy3", cids.at(1)); + std::move(callback).Run(true); + })); + + auto scoped_override = OverrideWithTimeNow(base::Time::FromTimeT(123u)); + + mojom::BlockchainTokenPtr token = + BraveWalletPinService::TokenFromPrefPath(kMonkey5Path); + token->is_erc721 = true; + absl::optional call_status; + service()->AddPin( + std::move(token), absl::nullopt, + base::BindLambdaForTesting( + [&call_status](bool result, mojom::PinErrorPtr error) { + call_status = result; + EXPECT_FALSE(error); + })); + EXPECT_TRUE(call_status.value()); + + const base::Value::Dict* token_record = + GetPrefs() + ->GetDict(kPinnedNFTAssets) + .FindDictByDottedPath(kMonkey5Path); + + base::Value::List expected_cids; + expected_cids.Append("QmeSjSinHpPnmXmspMjwiXyN6zS4E9zccariGR3jxcaWtq"); + expected_cids.Append("bafy3"); + + EXPECT_EQ(BraveWalletPinService::StatusToString( + mojom::TokenPinStatusCode::STATUS_PINNED), + *(token_record->FindString("status"))); + EXPECT_EQ(nullptr, token_record->FindDict("error")); + EXPECT_EQ(expected_cids, *(token_record->FindList("cids"))); + EXPECT_EQ(base::Time::FromTimeT(123u), + base::ValueToTime(token_record->Find("validate_timestamp"))); + } + + // Wrong gateway + { + ON_CALL(*GetJsonRpcService(), GetERC721Metadata(_, _, _, _)) + .WillByDefault(::testing::Invoke( + [](const std::string& contract_address, const std::string& token_id, + const std::string& chain_id, + MockJsonRpcService::GetERC721MetadataCallback callback) { + EXPECT_EQ("0x1", chain_id); + EXPECT_EQ("0xbc4ca0eda7647a8ab7c2061c2e118a18a936f13d", + contract_address); + EXPECT_EQ("0x5", token_id); + std::move(callback).Run(kMonkey6Url, kMonkey6, + mojom::ProviderError::kSuccess, ""); + })); + + mojom::BlockchainTokenPtr token = + BraveWalletPinService::TokenFromPrefPath(kMonkey6Path); + token->is_erc721 = true; + absl::optional call_status; + service()->AddPin( + std::move(token), absl::nullopt, + base::BindLambdaForTesting( + [&call_status](bool result, mojom::PinErrorPtr error) { + call_status = result; + EXPECT_TRUE(error); + })); + + EXPECT_FALSE(call_status.value()); + + const base::Value::Dict* token_record = + GetPrefs() + ->GetDict(kPinnedNFTAssets) + .FindDictByDottedPath(kMonkey6Path); + + EXPECT_EQ(BraveWalletPinService::StatusToString( + mojom::TokenPinStatusCode::STATUS_PINNING_FAILED), + *(token_record->FindString("status"))); + EXPECT_EQ(BraveWalletPinService::ErrorCodeToString( + mojom::WalletPinServiceErrorCode::ERR_NON_IPFS_TOKEN_URL), + token_record->FindByDottedPath("error.error_code")->GetString()); + } +} + TEST_F(BraveWalletPinServiceTest, AddPin_ContentVerification) { { ON_CALL(*GetContentTypeChecker(), CheckContentTypeSupported(_, _)) @@ -471,7 +607,7 @@ TEST_F(BraveWalletPinServiceTest, AddPin_ContentVerification) { } } -TEST_F(BraveWalletPinServiceTest, AddPin_NonIpfsImage) { +TEST_F(BraveWalletPinServiceTest, _NonIpfsImage) { { ON_CALL(*GetJsonRpcService(), GetERC721Metadata(_, _, _, _)) .WillByDefault(::testing::Invoke( From c68c605998a7abad5bce31342ea422da82cab118 Mon Sep 17 00:00:00 2001 From: oisupov Date: Tue, 7 Mar 2023 02:52:24 +0400 Subject: [PATCH 2/4] Show only pinned NFTs count in the IPFS banner https://github.com/brave/brave-browser/issues/28928 --- .../brave_wallet_ui/common/hooks/nft-pin.ts | 16 ++++++++++++++-- .../desktop/nft-ipfs-banner/nft-ipfs-banner.tsx | 7 +++---- .../nft-pinning-status/nft-pinning-status.tsx | 1 + components/brave_wallet_ui/stories/locale.ts | 8 ++++---- components/resources/wallet_strings.grdp | 2 +- 5 files changed, 23 insertions(+), 11 deletions(-) diff --git a/components/brave_wallet_ui/common/hooks/nft-pin.ts b/components/brave_wallet_ui/common/hooks/nft-pin.ts index 3c029ad90aee..b58d3150ddc5 100644 --- a/components/brave_wallet_ui/common/hooks/nft-pin.ts +++ b/components/brave_wallet_ui/common/hooks/nft-pin.ts @@ -51,10 +51,22 @@ export function useNftPin () { }, 0) }, [nftsPinningStatus]) + const inProgressNftCount = React.useMemo(() => { + return Object.keys(nftsPinningStatus).reduce((accumulator, currentValue) => { + const status = nftsPinningStatus[currentValue] + if (status?.code === BraveWallet.TokenPinStatusCode.STATUS_PINNING_IN_PROGRESS || + status?.code === BraveWallet.TokenPinStatusCode.STATUS_PINNING_PENDING) { + return accumulator += 1 + } + + return accumulator + }, 0) + }, [nftsPinningStatus]) + const pinningStatusSummary: BraveWallet.TokenPinStatusCode = React.useMemo(() => { - if (pinnableNfts.length === pinnedNftsCount) { + if (pinnedNftsCount > 0 && inProgressNftCount == 0) { return BraveWallet.TokenPinStatusCode.STATUS_PINNED - } else if (pinnableNfts.length > pinnedNftsCount) { + } else if (inProgressNftCount > 0) { return BraveWallet.TokenPinStatusCode.STATUS_PINNING_IN_PROGRESS } diff --git a/components/brave_wallet_ui/components/desktop/nft-ipfs-banner/nft-ipfs-banner.tsx b/components/brave_wallet_ui/components/desktop/nft-ipfs-banner/nft-ipfs-banner.tsx index 12cb82ec2026..d7fbb4180f25 100644 --- a/components/brave_wallet_ui/components/desktop/nft-ipfs-banner/nft-ipfs-banner.tsx +++ b/components/brave_wallet_ui/components/desktop/nft-ipfs-banner/nft-ipfs-banner.tsx @@ -44,7 +44,7 @@ export const NftIpfsBanner = ({ onDismiss }: Props) => { // memos const bannerStatus: BannerStatus = React.useMemo(() => { - if (!isAutoPinEnabled || pinnableNftsCount === 0) return 'start' + if (!isAutoPinEnabled) return 'start' switch (status) { case BraveWallet.TokenPinStatusCode.STATUS_PINNED: @@ -52,7 +52,7 @@ export const NftIpfsBanner = ({ onDismiss }: Props) => { case BraveWallet.TokenPinStatusCode.STATUS_PINNING_IN_PROGRESS: return 'uploading' default: - return 'success' + return 'start' } }, [status, pinnableNftsCount, isAutoPinEnabled]) @@ -79,8 +79,7 @@ export const NftIpfsBanner = ({ onDismiss }: Props) => { ) : bannerStatus === 'success' ? ( `${getLocale('braveWalletNftPinningBannerSuccess') - .replace('$1', `${pinnedNftsCount}`) - .replace('$2', `${pinnableNftsCount}`)}` + .replace('$1', `${pinnedNftsCount}`)}` ) : ( `${getLocale('braveWalletNftPinningBannerUploading')}` )} diff --git a/components/brave_wallet_ui/components/desktop/nft-pinning-status/nft-pinning-status.tsx b/components/brave_wallet_ui/components/desktop/nft-pinning-status/nft-pinning-status.tsx index 0b6595d4e916..feec82e88c88 100644 --- a/components/brave_wallet_ui/components/desktop/nft-pinning-status/nft-pinning-status.tsx +++ b/components/brave_wallet_ui/components/desktop/nft-pinning-status/nft-pinning-status.tsx @@ -30,6 +30,7 @@ export const NftPinningStatus = (props: Props) => { React.useEffect(() => { switch (pinningStatusCode) { case BraveWallet.TokenPinStatusCode.STATUS_PINNING_IN_PROGRESS: + case BraveWallet.TokenPinStatusCode.STATUS_PINNING_PENDING: setmessage(getLocale('braveWalletNftPinningStatusPinning')) setIcon() break diff --git a/components/brave_wallet_ui/stories/locale.ts b/components/brave_wallet_ui/stories/locale.ts index 2fc73d3f0dd4..4d2e498580e2 100644 --- a/components/brave_wallet_ui/stories/locale.ts +++ b/components/brave_wallet_ui/stories/locale.ts @@ -857,15 +857,15 @@ provideStrings({ braveWalletNftPinningCheckNftsButton: 'Check which NFTs of mine can be pinned', braveWalletNftPinningBannerStart: 'Now you can run your IPFS and be part of web 3. Your NFT data will stay online forever and cannot be tampered with.', braveWalletNftPinningBannerUploading: 'You\’re running IPFS node. File is being uploaded to IPFS.', - braveWalletNftPinningBannerSuccess: '$1 out of $2 NFTs have been successfully pinned to IPFS.', + braveWalletNftPinningBannerSuccess: '$1 NFTs have been successfully pinned to IPFS.', braveWalletNftPinningBannerLearnMore: 'Learn more', braveWalletNftPinningInspectHeading: '$1 out of $2 are available!', braveWalletNftPinningUnableToPin: 'Unable to pin', braveWalletNftPinningNodeRunningStatus: 'You\’re running IPFS node', braveWalletNftPinningNodeNotRunningStatus: 'Local IPFS node is not running', - braveWalletNftPinningStatusPinned: 'NFT data is being pinned to your local IPFS node', - braveWalletNftPinningStatusPinning: 'Cannot be pinned to your local IPFS node', - braveWalletNftPinningStatusPinningFailed: 'Pinned to your local IPFS node', + braveWalletNftPinningStatusPinned: 'Pinned to your local IPFS node', + braveWalletNftPinningStatusPinning: 'NFT data is being pinned to your local IPFS node', + braveWalletNftPinningStatusPinningFailed: 'Cannot be pinned to your local IPFS node', braveWalletImportNftModalTitle: 'Import NFT', braveWalletEditNftModalTitle: 'Edit NFT' }) diff --git a/components/resources/wallet_strings.grdp b/components/resources/wallet_strings.grdp index 16d29269711d..f30537c90835 100644 --- a/components/resources/wallet_strings.grdp +++ b/components/resources/wallet_strings.grdp @@ -855,7 +855,7 @@ Check which NFTs of mine can be pinned Now you can run your IPFS and be part of web 3. Your NFT data will stay online forever and cannot be tampered with. You’re running IPFS node. File is being fetched to local IPFS node. - $11 out of $23 NFTs have been successfully pinned to IPFS. + $11 NFTs have been successfully pinned to IPFS. Learn more $11 out of $23 are available! Unable to pin From fda1ae07435b3426073df0efb494d07c71da05f1 Mon Sep 17 00:00:00 2001 From: oisupov Date: Tue, 7 Mar 2023 20:35:30 +0400 Subject: [PATCH 3/4] Build fix --- components/brave_wallet_ui/common/hooks/nft-pin.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/brave_wallet_ui/common/hooks/nft-pin.ts b/components/brave_wallet_ui/common/hooks/nft-pin.ts index b58d3150ddc5..316d1106317c 100644 --- a/components/brave_wallet_ui/common/hooks/nft-pin.ts +++ b/components/brave_wallet_ui/common/hooks/nft-pin.ts @@ -64,7 +64,7 @@ export function useNftPin () { }, [nftsPinningStatus]) const pinningStatusSummary: BraveWallet.TokenPinStatusCode = React.useMemo(() => { - if (pinnedNftsCount > 0 && inProgressNftCount == 0) { + if (pinnedNftsCount > 0 && inProgressNftCount === 0) { return BraveWallet.TokenPinStatusCode.STATUS_PINNED } else if (inProgressNftCount > 0) { return BraveWallet.TokenPinStatusCode.STATUS_PINNING_IN_PROGRESS From f53932663d083966522bfb4b60162c839fdc5ae5 Mon Sep 17 00:00:00 2001 From: oisupov Date: Thu, 9 Mar 2023 02:00:04 +0400 Subject: [PATCH 4/4] Review fix --- .../browser/brave_wallet_pin_service.cc | 14 +++----------- .../browser/brave_wallet_pin_service_unittest.cc | 2 +- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/components/brave_wallet/browser/brave_wallet_pin_service.cc b/components/brave_wallet/browser/brave_wallet_pin_service.cc index 782619920c08..b59a50a5f428 100644 --- a/components/brave_wallet/browser/brave_wallet_pin_service.cc +++ b/components/brave_wallet/browser/brave_wallet_pin_service.cc @@ -96,12 +96,12 @@ absl::optional ExtractIpfsUrl(const std::string& url) { return gurl.spec(); } - auto scope = ipfs::ExtractSourceFromGateway(gurl); - if (!scope || !scope->SchemeIs(ipfs::kIPFSScheme)) { + auto source = ipfs::ExtractSourceFromGateway(gurl); + if (!source || !source->SchemeIs(ipfs::kIPFSScheme)) { return absl::nullopt; } - return scope.value().spec(); + return source.value().spec(); } absl::optional ExtractCID(const std::string& url) { @@ -111,14 +111,6 @@ absl::optional ExtractCID(const std::string& url) { return absl::nullopt; } - if (!gurl.SchemeIs(ipfs::kIPFSScheme)) { - auto scope = ipfs::ExtractSourceFromGateway(gurl); - if (!scope || !scope->SchemeIs(ipfs::kIPFSScheme)) { - return absl::nullopt; - } - return absl::nullopt; - } - std::vector result = base::SplitString( gurl.path(), "/", base::WhitespaceHandling::KEEP_WHITESPACE, base::SplitResult::SPLIT_WANT_NONEMPTY); diff --git a/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc b/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc index 276f8a243da0..4b411e119c34 100644 --- a/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc +++ b/components/brave_wallet/browser/brave_wallet_pin_service_unittest.cc @@ -607,7 +607,7 @@ TEST_F(BraveWalletPinServiceTest, AddPin_ContentVerification) { } } -TEST_F(BraveWalletPinServiceTest, _NonIpfsImage) { +TEST_F(BraveWalletPinServiceTest, AddPin_NonIpfsImage) { { ON_CALL(*GetJsonRpcService(), GetERC721Metadata(_, _, _, _)) .WillByDefault(::testing::Invoke(