diff --git a/components/brave_wallet/browser/eth_gas_utils.cc b/components/brave_wallet/browser/eth_gas_utils.cc index a79e22ba53bc..3734a90f2d0b 100644 --- a/components/brave_wallet/browser/eth_gas_utils.cc +++ b/components/brave_wallet/browser/eth_gas_utils.cc @@ -16,6 +16,40 @@ namespace brave_wallet { namespace eth { +// Scale the base fee by 33% to get a suggested value. This is done to account +// for sufficient fluctionations in the base fee, so that the transaction will +// not get stuck. +// +// A higher scaling factor increases the likelihood of confirmation within the +// next few blocks, but also makes the transaction appear more expensive to +// the user. +// +// Note that base fee is not part of the RLP. Any excees base fee is refunded, +// so the user will not be charged more than the base fee of the block that +// includes the transaction. +absl::optional ScaleBaseFeePerGas(const std::string& value) { + uint256_t value_uint256; + if (!HexValueToUint256(value, &value_uint256)) { + return absl::nullopt; + } + + // We use "double" math and this is unlikely to get hit, so return + // absl::nullopt if the value is too big. + if (value_uint256 > kMaxSafeIntegerUint64) { + return absl::nullopt; + } + + // Compiler crashes without these 2 casts :( + double value_double = + static_cast(static_cast(value_uint256)); + + value_double *= 1.33; + value_double = std::floor(value_double); + + // Compiler crashes without these 2 casts :( + return static_cast(static_cast(value_double)); +} + // Assumes there are 3 reward percentiles per element // The first for low, the second for avg, and the third for high. // The following calculations will be made: @@ -48,30 +82,12 @@ bool GetSuggested1559Fees(const std::vector& base_fee_per_gas, // "pending" is the last element in base_fee_per_gas // "latest" is the 2nd last element in base_fee_per_gas std::string pending_base_fee_per_gas = base_fee_per_gas.back(); - uint256_t pending_base_fee_per_gas_uint; - if (!HexValueToUint256(pending_base_fee_per_gas, - &pending_base_fee_per_gas_uint)) { - return false; - } - // We use "double" math and this is unlikely to get hit, so return false - // if the value is too big. - if (pending_base_fee_per_gas_uint > kMaxSafeIntegerUint64) { + auto scaled_base_fee_per_gas = ScaleBaseFeePerGas(pending_base_fee_per_gas); + if (!scaled_base_fee_per_gas) { return false; } - - // Compiler crashes without these 2 casts :( - double pending_base_fee_per_gas_dbl = - static_cast(static_cast(pending_base_fee_per_gas_uint)); - // The base fee is not part of the RLP. We only specify priority fee and max - // fee. So the excess will be refunded and it will be at most 33% It's best - // to assume a larger value here so there's a better chance it will get in the - // next block and if it's too high it will go back to the user - pending_base_fee_per_gas_dbl *= 1.33; - pending_base_fee_per_gas_dbl = std::floor(pending_base_fee_per_gas_dbl); - // Compiler crashes without these 2 casts :( - *suggested_base_fee_per_gas = - (uint256_t)(uint64_t)pending_base_fee_per_gas_dbl; + *suggested_base_fee_per_gas = *scaled_base_fee_per_gas; const uint256_t fallback_priority_fee = uint256_t(2e9); *low_priority_fee = fallback_priority_fee; diff --git a/components/brave_wallet/browser/eth_gas_utils.h b/components/brave_wallet/browser/eth_gas_utils.h index af9d8c7ddfc2..d6d7631e0371 100644 --- a/components/brave_wallet/browser/eth_gas_utils.h +++ b/components/brave_wallet/browser/eth_gas_utils.h @@ -15,6 +15,8 @@ namespace brave_wallet { namespace eth { +absl::optional ScaleBaseFeePerGas(const std::string& value); + bool GetSuggested1559Fees(const std::vector& base_fee_per_gas, const std::vector& gas_used_ratio, const std::string& oldest_block, diff --git a/components/brave_wallet/browser/eth_gas_utils_unittest.cc b/components/brave_wallet/browser/eth_gas_utils_unittest.cc index 97d69b2f3e9d..b19182887fc4 100644 --- a/components/brave_wallet/browser/eth_gas_utils_unittest.cc +++ b/components/brave_wallet/browser/eth_gas_utils_unittest.cc @@ -8,12 +8,28 @@ #include #include "brave/components/brave_wallet/browser/eth_gas_utils.h" +#include "brave/components/brave_wallet/common/hex_utils.h" #include "testing/gtest/include/gtest/gtest.h" namespace brave_wallet { namespace eth { +TEST(EthGasUtilsTest, ScaleBaseFeePerGas) { + // OK: scale 0x64 (100) + EXPECT_EQ(ScaleBaseFeePerGas("0x64"), 133ULL); + + // OK: scale 0x0 (0) + EXPECT_EQ(ScaleBaseFeePerGas("0x0"), 0ULL); + + // KO: invalid hex + EXPECT_FALSE(ScaleBaseFeePerGas("invalid")); + + // KO: value is too big + EXPECT_FALSE( + ScaleBaseFeePerGas(Uint256ValueToHex(kMaxSafeIntegerUint64 + 1))); +} + TEST(EthGasUtilsTest, GetSuggested1559Fees) { const std::vector base_fee_per_gas{ "0x257093e880", "0x20f4138789", "0x20b04643ea", diff --git a/components/brave_wallet/browser/eth_tx_manager.cc b/components/brave_wallet/browser/eth_tx_manager.cc index 3ef09af3b714..81e4ba9bd533 100644 --- a/components/brave_wallet/browser/eth_tx_manager.cc +++ b/components/brave_wallet/browser/eth_tx_manager.cc @@ -1136,19 +1136,30 @@ void EthTxManager::RetryTransaction(const std::string& chain_id, void EthTxManager::GetGasEstimation1559(const std::string& chain_id, GetGasEstimation1559Callback callback) { json_rpc_service_->GetFeeHistory( - chain_id, - base::BindOnce(&EthTxManager::OnGetGasEstimation1559, - weak_factory_.GetWeakPtr(), std::move(callback))); + chain_id, base::BindOnce(&EthTxManager::OnGetGasEstimation1559, + weak_factory_.GetWeakPtr(), std::move(callback), + chain_id)); } void EthTxManager::OnGetGasEstimation1559( GetGasEstimation1559Callback callback, + const std::string& chain_id, const std::vector& base_fee_per_gas, const std::vector& gas_used_ratio, const std::string& oldest_block, const std::vector>& reward, mojom::ProviderError error, const std::string& error_message) { + // If eth_feeHistory method was not found, try to get the base fee + // from eth_getBlockByNumber. + if (error == mojom::ProviderError::kMethodNotFound) { + json_rpc_service_->GetBaseFeePerGas( + chain_id, + base::BindOnce(&EthTxManager::OnGetBaseFeePerGas, + weak_factory_.GetWeakPtr(), std::move(callback))); + return; + } + if (error != mojom::ProviderError::kSuccess) { std::move(callback).Run(nullptr); return; @@ -1183,6 +1194,36 @@ void EthTxManager::OnGetGasEstimation1559( std::move(callback).Run(std::move(estimation)); } +void EthTxManager::OnGetBaseFeePerGas(GetGasEstimation1559Callback callback, + const std::string& base_fee_per_gas, + mojom::ProviderError error, + const std::string& error_message) { + if (base_fee_per_gas.empty()) { + std::move(callback).Run(nullptr); + return; + } + + mojom::GasEstimation1559Ptr estimation = mojom::GasEstimation1559::New(); + auto scaled_base_fee_per_gas_uint256 = + eth::ScaleBaseFeePerGas(base_fee_per_gas); + if (!scaled_base_fee_per_gas_uint256) { + std::move(callback).Run(nullptr); + return; + } + + const auto& scaled_base_fee_per_gas = + Uint256ValueToHex(*scaled_base_fee_per_gas_uint256); + + estimation->base_fee_per_gas = scaled_base_fee_per_gas; + estimation->slow_max_priority_fee_per_gas = "0x0"; + estimation->avg_max_priority_fee_per_gas = "0x0"; + estimation->fast_max_priority_fee_per_gas = "0x0"; + estimation->slow_max_fee_per_gas = scaled_base_fee_per_gas; + estimation->avg_max_fee_per_gas = scaled_base_fee_per_gas; + estimation->fast_max_fee_per_gas = scaled_base_fee_per_gas; + std::move(callback).Run(std::move(estimation)); +} + void EthTxManager::Reset() { TxManager::Reset(); pending_tx_tracker_->Reset(); diff --git a/components/brave_wallet/browser/eth_tx_manager.h b/components/brave_wallet/browser/eth_tx_manager.h index b3482c4588ce..75e80e5470b3 100644 --- a/components/brave_wallet/browser/eth_tx_manager.h +++ b/components/brave_wallet/browser/eth_tx_manager.h @@ -228,6 +228,7 @@ class EthTxManager : public TxManager, public EthBlockTracker::Observer { const std::string& error_message); void OnGetGasEstimation1559( GetGasEstimation1559Callback callback, + const std::string& chain_id, const std::vector& base_fee_per_gas, const std::vector& gas_used_ratio, const std::string& oldest_block, @@ -286,6 +287,11 @@ class EthTxManager : public TxManager, public EthBlockTracker::Observer { mojom::ProviderErrorUnionPtr error_union, const std::string& error_message); + void OnGetBaseFeePerGas(GetGasEstimation1559Callback callback, + const std::string& base_fee_per_gas, + mojom::ProviderError error, + const std::string& error_message); + // EthBlockTracker::Observer: void OnLatestBlock(const std::string& chain_id, uint256_t block_num) override {} diff --git a/components/brave_wallet/browser/eth_tx_manager_unittest.cc b/components/brave_wallet/browser/eth_tx_manager_unittest.cc index ee4bd910c41d..815ea906eb2f 100644 --- a/components/brave_wallet/browser/eth_tx_manager_unittest.cc +++ b/components/brave_wallet/browser/eth_tx_manager_unittest.cc @@ -1477,6 +1477,186 @@ TEST_F(EthTxManagerUnitTest, GetMojomGasEstimation())); } +TEST_F(EthTxManagerUnitTest, + AddUnapproved1559TransactionFeeHistoryNotFoundWithGasLimit) { + url_loader_factory_.SetInterceptor(base::BindLambdaForTesting( + [this](const network::ResourceRequest& request) { + url_loader_factory_.ClearResponses(); + std::string header_value; + EXPECT_TRUE(request.headers.GetHeader("X-Eth-Method", &header_value)); + if (header_value == "eth_getBlockByNumber") { + url_loader_factory_.AddResponse(request.url.spec(), R"( + { + "jsonrpc": "2.0", + "result": { + "baseFeePerGas": "0x64" + }, + "id": 1 + })"); + } else if (header_value == "eth_feeHistory") { + url_loader_factory_.AddResponse(request.url.spec(), R"( + { + "jsonrpc": "2.0", + "error": { + "code": -32601, + "message": "Method not found" + }, + "id": 1 + } + )"); + } + })); + + const std::string gas_limit = "0x974"; + auto tx_data = mojom::TxData1559::New( + mojom::TxData::New("0x1", "", gas_limit, + "0xbe862ad9abfe6f22bcb087716c7d89a26051f74c", + "0x016345785d8a0000", data_, false, absl::nullopt), + "0x04", "", "", nullptr); + + bool callback_called = false; + std::string tx_meta_id; + + AddUnapproved1559Transaction( + mojom::kLocalhostChainId, std::move(tx_data), from(), + base::BindOnce(&AddUnapprovedTransactionSuccessCallback, &callback_called, + &tx_meta_id)); + + base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(callback_called); + auto tx_meta = + eth_tx_manager()->GetTxForTesting(mojom::kLocalhostChainId, tx_meta_id); + EXPECT_TRUE(tx_meta); + + EXPECT_EQ(Uint256ValueToHex(tx_meta->tx()->gas_limit()), gas_limit); + auto* tx1559 = static_cast(tx_meta->tx()); + EXPECT_EQ(tx1559->max_priority_fee_per_gas(), 0ULL); + EXPECT_EQ(tx1559->max_fee_per_gas(), 133ULL); // 0x64 x 1.33 + + auto estimation = + mojom::GasEstimation1559::New("0x0", // slow_max_priority_fee_per_gas + "0x85", // slow_max_fee_per_gas + "0x0", // avg_max_priority_fee_per_gas + "0x85", // avg_max_fee_per_gas + "0x0", // fast_max_priority_fee_per_gas + "0x85", // fast_max_fee_per_gas + "0x85"); // base_fee_per_gas (0x64 x 1.33) + EXPECT_EQ(tx1559->gas_estimation(), + Eip1559Transaction::GasEstimation::FromMojomGasEstimation1559( + std::move(estimation))); +} + +TEST_F(EthTxManagerUnitTest, + AddUnapproved1559TransactionFeeHistoryNotFoundWithoutGasLimit) { + url_loader_factory_.SetInterceptor(base::BindLambdaForTesting( + [this](const network::ResourceRequest& request) { + url_loader_factory_.ClearResponses(); + std::string header_value; + EXPECT_TRUE(request.headers.GetHeader("X-Eth-Method", &header_value)); + if (header_value == "eth_getBlockByNumber") { + url_loader_factory_.AddResponse(request.url.spec(), R"( + { + "jsonrpc": "2.0", + "result": { + "baseFeePerGas": "0x64" + }, + "id": 1 + })"); + } else if (header_value == "eth_feeHistory") { + url_loader_factory_.AddResponse(request.url.spec(), R"( + { + "jsonrpc": "2.0", + "error": { + "code": -32601, + "message": "Method not found" + }, + "id": 1 + } + )"); + } else if (header_value == "eth_estimateGas") { + url_loader_factory_.AddResponse(request.url.spec(), R"( + { + "jsonrpc": "2.0", + "result": "0x00000000000009604", + "id": 1 + })"); + } + })); + + auto tx_data = mojom::TxData1559::New( + mojom::TxData::New("0x1", "", "", + "0xbe862ad9abfe6f22bcb087716c7d89a26051f74c", + "0x016345785d8a0000", data_, false, absl::nullopt), + "0x04", "", "", nullptr); + + bool callback_called = false; + std::string tx_meta_id; + + AddUnapproved1559Transaction( + mojom::kLocalhostChainId, std::move(tx_data), from(), + base::BindOnce(&AddUnapprovedTransactionSuccessCallback, &callback_called, + &tx_meta_id)); + + base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(callback_called); + auto tx_meta = + eth_tx_manager()->GetTxForTesting(mojom::kLocalhostChainId, tx_meta_id); + EXPECT_TRUE(tx_meta); + + EXPECT_EQ(Uint256ValueToHex(tx_meta->tx()->gas_limit()), "0x9604"); + auto* tx1559 = static_cast(tx_meta->tx()); + EXPECT_EQ(tx1559->max_priority_fee_per_gas(), 0ULL); + EXPECT_EQ(tx1559->max_fee_per_gas(), 133ULL); // 0x64 x 1.33 + + auto estimation = + mojom::GasEstimation1559::New("0x0", // slow_max_priority_fee_per_gas + "0x85", // slow_max_fee_per_gas + "0x0", // avg_max_priority_fee_per_gas + "0x85", // avg_max_fee_per_gas + "0x0", // fast_max_priority_fee_per_gas + "0x85", // fast_max_fee_per_gas + "0x85"); // base_fee_per_gas (0x64 x 1.33) + EXPECT_EQ(tx1559->gas_estimation(), + Eip1559Transaction::GasEstimation::FromMojomGasEstimation1559( + std::move(estimation))); +} + +TEST_F(EthTxManagerUnitTest, AddUnapproved1559TransactionFeeHistoryFailed) { + url_loader_factory_.SetInterceptor(base::BindLambdaForTesting( + [this](const network::ResourceRequest& request) { + url_loader_factory_.ClearResponses(); + std::string header_value; + EXPECT_TRUE(request.headers.GetHeader("X-Eth-Method", &header_value)); + if (header_value == "eth_feeHistory") { + url_loader_factory_.AddResponse(request.url.spec(), R"( + { + "jsonrpc": "2.0", + "error": { + "code": -32600, + "message": "Invalid request" + }, + "id": 1 + } + )"); + } + })); + + auto tx_data = mojom::TxData1559::New( + mojom::TxData::New("0x1", "", "0x9604", + "0xbe862ad9abfe6f22bcb087716c7d89a26051f74c", + "0x016345785d8a0000", data_, false, absl::nullopt), + "0x04", "", "", nullptr); + + bool callback_called = false; + AddUnapproved1559Transaction( + mojom::kLocalhostChainId, std::move(tx_data), from(), + base::BindOnce(&AddUnapprovedTransactionFailureCallback, + &callback_called)); + + base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(callback_called); +} + TEST_F(EthTxManagerUnitTest, AddUnapproved1559TransactionWithoutGasFeeAndLimitForEthSend) { auto tx_data = mojom::TxData1559::New( diff --git a/components/brave_wallet/browser/json_rpc_service.cc b/components/brave_wallet/browser/json_rpc_service.cc index 1463eb826f01..d449ba82e4b6 100644 --- a/components/brave_wallet/browser/json_rpc_service.cc +++ b/components/brave_wallet/browser/json_rpc_service.cc @@ -611,19 +611,20 @@ void JsonRpcService::MaybeUpdateIsEip1559(const std::string& chain_id) { return; } - GetIsEip1559(chain_id, - base::BindOnce(&JsonRpcService::UpdateIsEip1559, - weak_ptr_factory_.GetWeakPtr(), chain_id)); + GetBaseFeePerGas(chain_id, + base::BindOnce(&JsonRpcService::UpdateIsEip1559, + weak_ptr_factory_.GetWeakPtr(), chain_id)); } void JsonRpcService::UpdateIsEip1559(const std::string& chain_id, - bool is_eip1559, + const std::string& base_fee_per_gas, mojom::ProviderError error, const std::string& error_message) { if (error != mojom::ProviderError::kSuccess) { return; } + bool is_eip1559 = !base_fee_per_gas.empty(); bool changed = false; if (chain_id == brave_wallet::mojom::kLocalhostChainId) { changed = prefs_->GetBoolean(kSupportEip1559OnLocalhostChain) != is_eip1559; @@ -2104,21 +2105,26 @@ void JsonRpcService::OnGetGasPrice(GetGasPriceCallback callback, std::move(callback).Run(*result, mojom::ProviderError::kSuccess, ""); } -void JsonRpcService::GetIsEip1559(const std::string& chain_id, - GetIsEip1559Callback callback) { +// Retrieves the BASEFEE per gas for a given chain ID. +// +// If the chain does not support EIP-1559, and assuming no other provider +// errors, an empty string as base fee with kSuccess error will be delivered +// to the provided callback. +void JsonRpcService::GetBaseFeePerGas(const std::string& chain_id, + GetBaseFeePerGasCallback callback) { auto internal_callback = - base::BindOnce(&JsonRpcService::OnGetIsEip1559, + base::BindOnce(&JsonRpcService::OnGetBaseFeePerGas, weak_ptr_factory_.GetWeakPtr(), std::move(callback)); RequestInternal(eth::eth_getBlockByNumber(kEthereumBlockTagLatest, false), true, GetNetworkURL(prefs_, chain_id, mojom::CoinType::ETH), std::move(internal_callback)); } -void JsonRpcService::OnGetIsEip1559(GetIsEip1559Callback callback, - APIRequestResult api_request_result) { +void JsonRpcService::OnGetBaseFeePerGas(GetBaseFeePerGasCallback callback, + APIRequestResult api_request_result) { if (!api_request_result.Is2XXResponseCode()) { std::move(callback).Run( - false, mojom::ProviderError::kInternalError, + "", mojom::ProviderError::kInternalError, l10n_util::GetStringUTF8(IDS_WALLET_INTERNAL_ERROR)); return; } @@ -2129,13 +2135,17 @@ void JsonRpcService::OnGetIsEip1559(GetIsEip1559Callback callback, std::string error_message; ParseErrorResult(api_request_result.value_body(), &error, &error_message); - std::move(callback).Run(false, error, error_message); + std::move(callback).Run("", error, error_message); return; } const std::string* base_fee = result->FindString("baseFeePerGas"); - std::move(callback).Run(base_fee && !base_fee->empty(), - mojom::ProviderError::kSuccess, ""); + if (base_fee && !base_fee->empty()) { + std::move(callback).Run(*base_fee, mojom::ProviderError::kSuccess, ""); + } else { + // Successful response, but the chain does not support EIP-1559. + std::move(callback).Run("", mojom::ProviderError::kSuccess, ""); + } } void JsonRpcService::GetBlockByNumber(const std::string& chain_id, diff --git a/components/brave_wallet/browser/json_rpc_service.h b/components/brave_wallet/browser/json_rpc_service.h index 7990ceb62088..6d6356eebc04 100644 --- a/components/brave_wallet/browser/json_rpc_service.h +++ b/components/brave_wallet/browser/json_rpc_service.h @@ -315,11 +315,12 @@ class JsonRpcService : public KeyedService, public mojom::JsonRpcService { const std::string& error_message)>; void GetGasPrice(const std::string& chain_id, GetGasPriceCallback callback); - using GetIsEip1559Callback = - base::OnceCallback; - void GetIsEip1559(const std::string& chain_id, GetIsEip1559Callback callback); + void GetBaseFeePerGas(const std::string& chain_id, + GetBaseFeePerGasCallback callback); using GetBlockByNumberCallback = base::OnceCallbackGetIsEip1559( + json_rpc_service_->GetBaseFeePerGas( mojom::kLocalhostChainId, - base::BindOnce(&OnBoolResponse, &callback_called, - mojom::ProviderError::kSuccess, "", true)); + base::BindOnce(&OnStringResponse, &callback_called, + mojom::ProviderError::kSuccess, "", "0x181f22e7a9")); base::RunLoop().RunUntilIdle(); EXPECT_TRUE(callback_called); // Successful path when the network is not EIP1559 callback_called = false; SetIsEip1559Interceptor(expected_network, false); - json_rpc_service_->GetIsEip1559( + json_rpc_service_->GetBaseFeePerGas( mojom::kLocalhostChainId, - base::BindOnce(&OnBoolResponse, &callback_called, - mojom::ProviderError::kSuccess, "", false)); + base::BindOnce(&OnStringResponse, &callback_called, + mojom::ProviderError::kSuccess, "", "")); base::RunLoop().RunUntilIdle(); EXPECT_TRUE(callback_called); callback_called = false; SetHTTPRequestTimeoutInterceptor(); - json_rpc_service_->GetIsEip1559( + json_rpc_service_->GetBaseFeePerGas( mojom::kLocalhostChainId, - base::BindOnce(&OnBoolResponse, &callback_called, + base::BindOnce(&OnStringResponse, &callback_called, mojom::ProviderError::kInternalError, - l10n_util::GetStringUTF8(IDS_WALLET_INTERNAL_ERROR), - false)); + l10n_util::GetStringUTF8(IDS_WALLET_INTERNAL_ERROR), "")); base::RunLoop().RunUntilIdle(); EXPECT_TRUE(callback_called); callback_called = false; SetInvalidJsonInterceptor(); - json_rpc_service_->GetIsEip1559( + json_rpc_service_->GetBaseFeePerGas( mojom::kLocalhostChainId, - base::BindOnce(&OnBoolResponse, &callback_called, + base::BindOnce(&OnStringResponse, &callback_called, mojom::ProviderError::kParsingError, - l10n_util::GetStringUTF8(IDS_WALLET_PARSING_ERROR), - false)); + l10n_util::GetStringUTF8(IDS_WALLET_PARSING_ERROR), "")); base::RunLoop().RunUntilIdle(); EXPECT_TRUE(callback_called); callback_called = false; SetLimitExceededJsonErrorResponse(); - json_rpc_service_->GetIsEip1559( + json_rpc_service_->GetBaseFeePerGas( mojom::kLocalhostChainId, - base::BindOnce(&OnBoolResponse, &callback_called, + base::BindOnce(&OnStringResponse, &callback_called, mojom::ProviderError::kLimitExceeded, - "Request exceeds defined limit", false)); + "Request exceeds defined limit", "")); base::RunLoop().RunUntilIdle(); EXPECT_TRUE(callback_called); }