Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(wallet): fix EIP-1559 gas estimation issue with zksync (uplift to 1.53.x) #18771

Merged
merged 1 commit into from
Jun 9, 2023
Merged

fix(wallet): fix EIP-1559 gas estimation issue with zksync (uplift to 1.53.x) #18771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 37 additions & 21 deletions components/brave_wallet/browser/eth_gas_utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint256_t> 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<double>(static_cast<uint64_t>(value_uint256));

value_double *= 1.33;
value_double = std::floor(value_double);

// Compiler crashes without these 2 casts :(
return static_cast<uint256_t>(static_cast<uint64_t>(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:
Expand Down Expand Up @@ -48,30 +82,12 @@ bool GetSuggested1559Fees(const std::vector<std::string>& 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<double>(static_cast<uint64_t>(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;
Expand Down
2 changes: 2 additions & 0 deletions components/brave_wallet/browser/eth_gas_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ namespace brave_wallet {

namespace eth {

absl::optional<uint256_t> ScaleBaseFeePerGas(const std::string& value);

bool GetSuggested1559Fees(const std::vector<std::string>& base_fee_per_gas,
const std::vector<double>& gas_used_ratio,
const std::string& oldest_block,
Expand Down
16 changes: 16 additions & 0 deletions components/brave_wallet/browser/eth_gas_utils_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,28 @@
#include <vector>

#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<std::string> base_fee_per_gas{
"0x257093e880", "0x20f4138789", "0x20b04643ea",
Expand Down
47 changes: 44 additions & 3 deletions components/brave_wallet/browser/eth_tx_manager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string>& base_fee_per_gas,
const std::vector<double>& gas_used_ratio,
const std::string& oldest_block,
const std::vector<std::vector<std::string>>& 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;
Expand Down Expand Up @@ -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();
Expand Down
6 changes: 6 additions & 0 deletions components/brave_wallet/browser/eth_tx_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string>& base_fee_per_gas,
const std::vector<double>& gas_used_ratio,
const std::string& oldest_block,
Expand Down Expand Up @@ -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 {}
Expand Down
180 changes: 180 additions & 0 deletions components/brave_wallet/browser/eth_tx_manager_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Eip1559Transaction*>(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<Eip1559Transaction*>(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(
Expand Down
Loading