Skip to content

Commit

Permalink
Skip preflight checks for compressed NFT transfers (#24571)
Browse files Browse the repository at this point in the history
Unless preflight checks have been explicitly set to true by dapps.
  • Loading branch information
nvonpentz committed Jul 11, 2024
1 parent 5f64c50 commit 0def99a
Show file tree
Hide file tree
Showing 10 changed files with 217 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ std::optional<SolanaInstruction> Transfer(
const std::string log_wrapper = "noopb9bkMVfRPU8AsbpTUg8AQkHtKwMYZiFUjNRtMmV";

// Init instruction data with the instruction discriminator.
std::vector<uint8_t> instruction_data = {163, 52, 200, 231, 140, 3, 69, 186};
std::vector<uint8_t> instruction_data = kTransferInstructionDiscriminator;

std::vector<uint8_t> root_bytes;
if (!Base58Decode(proof.root, &root_bytes, kSolanaHashSize)) {
Expand Down
3 changes: 3 additions & 0 deletions components/brave_wallet/browser/solana_instruction_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ SolanaInstruction SetComputeUnitPrice(uint64_t price);

namespace bubblegum_program {

const std::vector<uint8_t> kTransferInstructionDiscriminator = {
163, 52, 200, 231, 140, 3, 69, 186};

// https://github.com/metaplex-foundation/mpl-bubblegum/blob/5b3cdfc6b236773be70dc1f0b0cb84badf881248/clients/js-solita/src/generated/instructions/transfer.ts#L81
std::optional<SolanaInstruction> Transfer(
uint32_t canopy_depth,
Expand Down
18 changes: 18 additions & 0 deletions components/brave_wallet/browser/solana_instruction_data_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "base/no_destructor.h"
#include "base/notreached.h"
#include "base/sys_byteorder.h"
#include "brave/components/brave_wallet/browser/solana_instruction_builder.h"
#include "brave/components/brave_wallet/common/brave_wallet_constants.h"
#include "brave/components/brave_wallet/common/encoding_utils.h"
#include "brave/components/brave_wallet/common/solana_utils.h"
Expand Down Expand Up @@ -938,4 +939,21 @@ GetComputeBudgetInstructionType(const std::vector<uint8_t>& data,
return mojo_ins_type;
}

bool IsCompressedNftTransferInstruction(const std::vector<uint8_t>& data,
const std::string& program_id) {
if (program_id != mojom::kSolanaBubbleGumProgramId) {
return false;
}

if (data.size() <
solana::bubblegum_program::kTransferInstructionDiscriminator.size()) {
return false;
}

return std::equal(
solana::bubblegum_program::kTransferInstructionDiscriminator.begin(),
solana::bubblegum_program::kTransferInstructionDiscriminator.end(),
data.begin());
}

} // namespace brave_wallet::solana_ins_data_decoder
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ std::optional<mojom::SolanaComputeBudgetInstruction>
GetComputeBudgetInstructionType(const std::vector<uint8_t>& data,
const std::string& program_id);

bool IsCompressedNftTransferInstruction(const std::vector<uint8_t>& data,
const std::string& program_id);

std::vector<InsParamPair> GetAccountParamsForTesting(
std::optional<mojom::SolanaSystemInstruction> sys_ins_type,
std::optional<mojom::SolanaTokenInstruction> token_ins_type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,4 +426,31 @@ TEST_F(SolanaInstructionDecoderTest, GetComputeBudgetInstructionType) {
ASSERT_FALSE(instruction_type);
}

TEST_F(SolanaInstructionDecoderTest, IsCompressedNftTransferInstruction) {
// Empty data is not a compressed NFT transfer instruction.
EXPECT_FALSE(
IsCompressedNftTransferInstruction({}, mojom::kSolanaBubbleGumProgramId));

// Compressed NFT transfer instruction is recognized.
std::vector<uint8_t> data = {
// Contains compressed NFT transfer disctiminator.
0xa3, 0x34, 0xc8, 0xe7, 0x8c, 0x03, 0x45, 0xba, 0x44, 0x3f, 0xca, 0x38,
0xd1, 0x3e, 0x68, 0xf2, 0x95, 0xaf, 0xfc, 0x5f, 0x34, 0x31, 0xf3, 0x75,
0xba, 0xd8, 0xd3, 0x82, 0x90, 0x1a, 0x94, 0x7f, 0x72, 0x96, 0xfc, 0xd8,
0x79, 0x8a, 0xb7, 0x98, 0x3b, 0x17, 0x52, 0x74, 0x15, 0x6f, 0x94, 0x1a,
0xe6, 0xc6, 0x1e, 0x0e, 0xb4, 0x6c, 0xcf, 0x64, 0xd6, 0x8f, 0xfd, 0x34,
0xb7, 0x68, 0x6d, 0x97, 0x32, 0x45, 0x7e, 0x8a, 0x5c, 0x1a, 0x80, 0x31,
0x9b, 0x22, 0x99, 0xb4, 0xc2, 0x20, 0x0e, 0x5e, 0xef, 0x2e, 0x12, 0xb1,
0x6d, 0x4f, 0xbd, 0xf1, 0x2e, 0x11, 0xe1, 0x4f, 0xb2, 0x76, 0xc3, 0x91,
0x21, 0x88, 0x34, 0xf3, 0x0a, 0xec, 0x39, 0x45, 0xa5, 0x15, 0x14, 0x00,
0x00, 0x00, 0x00, 0x00, 0xa5, 0x15, 0x14, 0x00};
EXPECT_TRUE(IsCompressedNftTransferInstruction(
data, mojom::kSolanaBubbleGumProgramId));

// Compressed NFT transfer instruction is not recognized for different program
// id.
EXPECT_FALSE(
IsCompressedNftTransferInstruction(data, mojom::kSolanaTokenProgramId));
}

} // namespace brave_wallet::solana_ins_data_decoder
11 changes: 11 additions & 0 deletions components/brave_wallet/browser/solana_message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,17 @@ bool SolanaMessage::UsesDurableNonce() const {
return true;
}

bool SolanaMessage::ContainsCompressedNftTransfer() const {
for (const auto& instruction : instructions_) {
if (solana_ins_data_decoder::IsCompressedNftTransferInstruction(
instruction.data(), instruction.GetProgramId())) {
return true;
}
}

return false;
}

bool SolanaMessage::UsesPriorityFee() const {
for (const auto& instruction : instructions_) {
auto instruction_type =
Expand Down
4 changes: 4 additions & 0 deletions components/brave_wallet/browser/solana_message.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ class SolanaMessage {
// https://docs.rs/solana-sdk/1.18.9/src/solana_sdk/transaction/versioned/mod.rs.html#192
bool UsesDurableNonce() const;

// Returns true if the message contains a compressed NFT transfer
// instruction.
bool ContainsCompressedNftTransfer() const;

// Returns whether the priority fee was added.
bool AddPriorityFee(uint32_t compute_units, uint64_t fee_per_compute_unit);

Expand Down
34 changes: 34 additions & 0 deletions components/brave_wallet/browser/solana_message_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/json/json_reader.h"
#include "base/sys_byteorder.h"
#include "base/test/gtest_util.h"
#include "brave/components/brave_wallet/browser/simple_hash_client.h"
#include "brave/components/brave_wallet/browser/solana_account_meta.h"
#include "brave/components/brave_wallet/browser/solana_instruction.h"
#include "brave/components/brave_wallet/browser/solana_instruction_builder.h"
Expand Down Expand Up @@ -728,4 +729,37 @@ TEST(SolanaMessageUnitTest, UsesPriorityFee) {
EXPECT_TRUE(message4.UsesPriorityFee());
}

TEST(SolanaMessageUnitTest, ContainsCompressedNftTransfer) {
// Legacy message does not contain compressed NFT transfer.
SolanaMessage message1 = GetTestLegacyMessage();
EXPECT_FALSE(message1.ContainsCompressedNftTransfer());

// Message with compressed NFT transfer instruction.
std::vector<mojom::SolanaAccountMetaPtr> account_metas;
std::vector<uint8_t> data = {
// Contains compressed NFT transfer disctiminator.
0xa3, 0x34, 0xc8, 0xe7, 0x8c, 0x03, 0x45, 0xba, 0x44, 0x3f, 0xca, 0x38,
0xd1, 0x3e, 0x68, 0xf2, 0x95, 0xaf, 0xfc, 0x5f, 0x34, 0x31, 0xf3, 0x75,
0xba, 0xd8, 0xd3, 0x82, 0x90, 0x1a, 0x94, 0x7f, 0x72, 0x96, 0xfc, 0xd8,
0x79, 0x8a, 0xb7, 0x98, 0x3b, 0x17, 0x52, 0x74, 0x15, 0x6f, 0x94, 0x1a,
0xe6, 0xc6, 0x1e, 0x0e, 0xb4, 0x6c, 0xcf, 0x64, 0xd6, 0x8f, 0xfd, 0x34,
0xb7, 0x68, 0x6d, 0x97, 0x32, 0x45, 0x7e, 0x8a, 0x5c, 0x1a, 0x80, 0x31,
0x9b, 0x22, 0x99, 0xb4, 0xc2, 0x20, 0x0e, 0x5e, 0xef, 0x2e, 0x12, 0xb1,
0x6d, 0x4f, 0xbd, 0xf1, 0x2e, 0x11, 0xe1, 0x4f, 0xb2, 0x76, 0xc3, 0x91,
0x21, 0x88, 0x34, 0xf3, 0x0a, 0xec, 0x39, 0x45, 0xa5, 0x15, 0x14, 0x00,
0x00, 0x00, 0x00, 0x00, 0xa5, 0x15, 0x14, 0x00};
auto solana_instruction = mojom::SolanaInstruction::New(
mojom::kSolanaBubbleGumProgramId, std::move(account_metas),
std::move(data), nullptr);

std::vector<mojom::SolanaInstructionPtr> mojom_instructions;
mojom_instructions.push_back(std::move(solana_instruction));
std::vector<SolanaInstruction> instructions;
SolanaInstruction::FromMojomSolanaInstructions(mojom_instructions,
&instructions);

message1.SetInstructionsForTesting(instructions);
EXPECT_TRUE(message1.ContainsCompressedNftTransfer());
}

} // namespace brave_wallet
18 changes: 18 additions & 0 deletions components/brave_wallet/browser/solana_tx_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,24 @@ void SolanaTxManager::AddUnapprovedTransaction(
meta->set_status(mojom::TransactionStatus::Unapproved);
meta->set_chain_id(chain_id);

// Skip preflight checks for compressed NFT transfers to avoid a potential
// Solana RPC bug that incorrectly shows compute budget exceeded, causing
// simulation failures.
if (meta->tx()->message()->ContainsCompressedNftTransfer()) {
auto options = meta->tx()->send_options();
if (options) {
if (options->skip_preflight == std::nullopt) {
// Only set skip_preflight to true if it's not already set because we
// want to respect the send options provided by dapps.
options->skip_preflight = true;
meta->tx()->set_send_options(options);
}
} else {
meta->tx()->set_send_options(
SolanaTransaction::SendOptions(std::nullopt, std::nullopt, true));
}
}

auto internal_callback =
base::BindOnce(&SolanaTxManager::ContinueAddUnapprovedTransaction,
weak_ptr_factory_.GetWeakPtr(), std::move(callback));
Expand Down
98 changes: 98 additions & 0 deletions components/brave_wallet/browser/solana_tx_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,104 @@ TEST_F(SolanaTxManagerUnitTest, AddAndApproveTransaction) {
SolanaSignatureStatus(72u, 0u, "", "finalized"));
}

TEST_F(SolanaTxManagerUnitTest, CompressedNftTransferSendOptions) {
// Non solana compressed NFT transfer transaction (empty data is missing
// transfer instruction discriminator).
std::vector<mojom::SolanaAccountMetaPtr> account_metas_non_compressed;
std::vector<uint8_t> data_non_compressed;
auto solana_instruction_non_compressed = mojom::SolanaInstruction::New(
mojom::kSolanaBubbleGumProgramId, std::move(account_metas_non_compressed),
std::move(data_non_compressed), nullptr);

std::vector<mojom::SolanaInstructionPtr> instructions_non_compressed;
instructions_non_compressed.push_back(
std::move(solana_instruction_non_compressed));

mojom::SolanaTxDataPtr solana_tx_data_non_compressed =
mojom::SolanaTxData::New(
"", 0, "FBG2vwk2tGKHbEWHSxf7rJGDuZ2eHaaNQ8u6c7xGt9Yv",
"4szaz6FsfBzwcCJYjbwZWEw3E8rKB4tz76644C8sAZo9", "", 0, 0,
mojom::TransactionType::SolanaCompressedNftTransfer,
std::move(instructions_non_compressed),
mojom::SolanaMessageVersion::kLegacy,
mojom::SolanaMessageHeader::New(1, 0, 30),
std::vector<std::string>({}),
std::vector<mojom::SolanaMessageAddressTableLookupPtr>(), nullptr,
nullptr, nullptr);
const auto& from_account = sol_account();

// Adding a non-compressed nft transfer should not add send options.
std::string meta_id1;
AddUnapprovedTransaction(mojom::kSolanaMainnet,
solana_tx_data_non_compressed.Clone(), from_account,
&meta_id1);
auto tx_meta1 = solana_tx_manager()->GetTxForTesting(meta_id1);
EXPECT_EQ(tx_meta1->tx()->send_options(), std::nullopt);

// Changing the transaction type to compressed nft transfer should add
// send options, and set skip_preflight to true.
std::vector<mojom::SolanaAccountMetaPtr> account_metas;
std::vector<uint8_t> data = {
// Has the right discriminator.
0xa3, 0x34, 0xc8, 0xe7, 0x8c, 0x03, 0x45, 0xba, 0x44, 0x3f, 0xca, 0x38,
0xd1, 0x3e, 0x68, 0xf2, 0x95, 0xaf, 0xfc, 0x5f, 0x34, 0x31, 0xf3, 0x75,
0xba, 0xd8, 0xd3, 0x82, 0x90, 0x1a, 0x94, 0x7f, 0x72, 0x96, 0xfc, 0xd8,
0x79, 0x8a, 0xb7, 0x98, 0x3b, 0x17, 0x52, 0x74, 0x15, 0x6f, 0x94, 0x1a,
0xe6, 0xc6, 0x1e, 0x0e, 0xb4, 0x6c, 0xcf, 0x64, 0xd6, 0x8f, 0xfd, 0x34,
0xb7, 0x68, 0x6d, 0x97, 0x32, 0x45, 0x7e, 0x8a, 0x5c, 0x1a, 0x80, 0x31,
0x9b, 0x22, 0x99, 0xb4, 0xc2, 0x20, 0x0e, 0x5e, 0xef, 0x2e, 0x12, 0xb1,
0x6d, 0x4f, 0xbd, 0xf1, 0x2e, 0x11, 0xe1, 0x4f, 0xb2, 0x76, 0xc3, 0x91,
0x21, 0x88, 0x34, 0xf3, 0x0a, 0xec, 0x39, 0x45, 0xa5, 0x15, 0x14, 0x00,
0x00, 0x00, 0x00, 0x00, 0xa5, 0x15, 0x14, 0x00};
auto solana_instruction = mojom::SolanaInstruction::New(
mojom::kSolanaBubbleGumProgramId, std::move(account_metas),
std::move(data), nullptr);

std::vector<mojom::SolanaInstructionPtr> instructions;
instructions.push_back(std::move(solana_instruction));

mojom::SolanaTxDataPtr solana_tx_data = mojom::SolanaTxData::New(
"", 0, "FBG2vwk2tGKHbEWHSxf7rJGDuZ2eHaaNQ8u6c7xGt9Yv",
"4szaz6FsfBzwcCJYjbwZWEw3E8rKB4tz76644C8sAZo9", "", 0, 0,
mojom::TransactionType::SolanaCompressedNftTransfer,
std::move(instructions), mojom::SolanaMessageVersion::kLegacy,
mojom::SolanaMessageHeader::New(1, 0, 30), std::vector<std::string>({}),
std::vector<mojom::SolanaMessageAddressTableLookupPtr>(), nullptr,
nullptr, nullptr);

std::string meta_id2;
AddUnapprovedTransaction(mojom::kSolanaMainnet, solana_tx_data.Clone(),
from_account, &meta_id2);
auto tx_meta2 = solana_tx_manager()->GetTxForTesting(meta_id2);
ASSERT_TRUE(tx_meta2->tx()->send_options());
ASSERT_TRUE(tx_meta2->tx()->send_options()->skip_preflight);
EXPECT_TRUE(tx_meta2->tx()->send_options()->skip_preflight.value());

// If send options are present but skip_preflight is null, it should be set to
// true.
solana_tx_data->send_options = mojom::SolanaSendTransactionOptions::New();
std::string meta_id3;
AddUnapprovedTransaction(mojom::kSolanaMainnet, solana_tx_data.Clone(),
from_account, &meta_id3);
auto tx_meta3 = solana_tx_manager()->GetTxForTesting(meta_id3);
ASSERT_TRUE(tx_meta3->tx()->send_options());
ASSERT_TRUE(tx_meta3->tx()->send_options()->skip_preflight);
EXPECT_TRUE(tx_meta3->tx()->send_options()->skip_preflight.value());

// If send options are present and skip_preflight is set to false, it should
// remain false.
solana_tx_data->send_options = mojom::SolanaSendTransactionOptions::New();
solana_tx_data->send_options->skip_preflight =
mojom::OptionalSkipPreflight::New(false);
std::string meta_id4;
AddUnapprovedTransaction(mojom::kSolanaMainnet, solana_tx_data.Clone(),
from_account, &meta_id4);
auto tx_meta4 = solana_tx_manager()->GetTxForTesting(meta_id4);
ASSERT_TRUE(tx_meta4->tx()->send_options());
ASSERT_TRUE(tx_meta4->tx()->send_options()->skip_preflight);
EXPECT_FALSE(tx_meta4->tx()->send_options()->skip_preflight.value());
}

TEST_F(SolanaTxManagerUnitTest, OfacSanctionedToAddress) {
const auto& from = sol_account();
const std::string ofac_sanctioned_to =
Expand Down

0 comments on commit 0def99a

Please sign in to comment.