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

Update Gnosis Chain Gas Estimation by 30% #1989

Merged
merged 9 commits into from
Aug 28, 2023
Merged

Update Gnosis Chain Gas Estimation by 30% #1989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
29f19d4
Add chain ids
elgatovital Aug 28, 2023
a91ae1b
Add string
elgatovital Aug 28, 2023
119d92d
Remove rinkeby
elgatovital Aug 28, 2023
b92ce67
Adjust gas estimation on GC
elgatovital Aug 28, 2023
69ea8e2
Adjust test
elgatovital Aug 28, 2023
acd2092
Cleanup
elgatovital Aug 28, 2023
c229681
Adjust tests
elgatovital Aug 28, 2023
0bd4426
Adjust tests
elgatovital Aug 28, 2023
aed9dc5
Adjust tests
elgatovital Aug 28, 2023
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
4 changes: 2 additions & 2 deletions app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,9 @@ class TxReviewFragment : BaseViewBindingFragment<FragmentTxReviewBinding>() {
requireView(),
getString(R.string.error_description_tx_exec_estimate) + ": ${it.cause.localizedMessage}"
)
is TxWillFail -> errorSnackbar(
is TxFails -> errorSnackbar(
requireView(),
getString(R.string.error_description_tx_exec_will_fail)
getString(R.string.error_description_tx_exec_fails)
)
is TxSumbitFailed -> errorSnackbar(
requireView(),
Expand Down
12 changes: 10 additions & 2 deletions app/src/main/java/io/gnosis/safe/ui/transactions/execution/TxReviewViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package io.gnosis.safe.ui.transactions.execution

import androidx.annotation.VisibleForTesting
import io.gnosis.data.backend.rpc.RpcClient
import io.gnosis.data.models.Chain
import io.gnosis.data.models.Owner
import io.gnosis.data.models.Safe
import io.gnosis.data.models.transaction.DetailedExecutionInfo
Expand Down Expand Up @@ -228,6 +229,13 @@ class TxReviewViewModel
nonce = minNonce
gasLimit = estimationParams.estimate

// fix for a bug in Nethermind requiring 30% increase in the gas estimation
// on gnosis chain for transactions with positive safeTxGas
if (activeSafe.chainId == Chain.ID_GNOSIS &&
(executionInfo as DetailedExecutionInfo.MultisigExecutionDetails).safeTxGas > BigInteger.ZERO) {
gasLimit = gasLimit!!.multiply(BigInteger.valueOf(130)).divide(BigInteger.valueOf(100))
}

if (isLegacy()) {
gasPrice = Wei(baseFee).toGWei(activeSafe.chain.currency.decimals)
} else {
Expand All @@ -246,7 +254,7 @@ class TxReviewViewModel
if (totalFeeValue() ?: BigInteger.ZERO > estimationParams.balance) {
throw InsufficientExecutionBalance
} else if (!estimationParams.callResult) {
throw TxWillFail
throw TxFails
}

}.onFailure {
Expand Down Expand Up @@ -516,7 +524,7 @@ class TxEstimationFailed(override val cause: Throwable) : Throwable(cause)

class TxSumbitFailed(override val cause: Throwable) : Throwable(cause)

object TxWillFail : Throwable()
object TxFails : Throwable()

object InsufficientExecutionBalance : Throwable()

Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
<string name="error_description_tx_exec_balances">Failed to load balances</string>
<string name="error_description_tx_exec_insufficient_balance">Insufficient balance for network fees</string>
<string name="error_description_tx_exec_will_fail">This transaction will most likely fail. To save gas costs, avoid executing the transaction.</string>
<string name="error_description_tx_exec_fails">Internal Safe transaction fails. Please double check whether this transaction is valid with the current state of the Safe.</string>
<string name="error_description_tx_exec_estimate">Estimation failed</string>
<string name="error_description_tx_exec_submit">Failed to submit transaction</string>

Expand Down
4 changes: 2 additions & 2 deletions app/src/test/java/io/gnosis/safe/ui/safe/add/AddSafeNameViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ class AddSafeNameViewModelTest {
listOf()
)
private val rinkeby = Chain(
Chain.ID_RINKEBY,
Chain.ID_GOERLI,
true,
"Rinkeby",
"Goerli",
"",
"",
"",
Expand Down
15 changes: 12 additions & 3 deletions app/src/test/java/io/gnosis/safe/ui/transactions/details/ConfirmRejectionViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,19 @@ import io.gnosis.data.repositories.SafeRepository
import io.gnosis.data.repositories.TransactionRepository
import io.gnosis.data.utils.SemVer
import io.gnosis.data.utils.calculateSafeTxHash
import io.gnosis.safe.*
import io.gnosis.safe.TestLifecycleRule
import io.gnosis.safe.TestLiveDataObserver
import io.gnosis.safe.Tracker
import io.gnosis.safe.appDispatchers
import io.gnosis.safe.readJsonFrom
import io.gnosis.safe.test
import io.gnosis.safe.ui.base.BaseStateViewModel
import io.gnosis.safe.ui.settings.app.SettingsHandler
import io.mockk.*
import io.mockk.Runs
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.just
import io.mockk.mockk
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.runTest
import org.junit.Assert.assertEquals
Expand Down Expand Up @@ -141,7 +150,7 @@ class ConfirmRejectionViewModelTest {
val rejectionTxHash =
calculateSafeTxHash(
implementationVersion = SemVer(1, 1, 0),
chainId = Chain.ID_RINKEBY,
chainId = Chain.ID_GOERLI,
safeAddress = safeAddress,
transaction = rejectionTxDetails,
executionInfo = rejectionExecutionInfo
Expand Down
10 changes: 7 additions & 3 deletions data/src/main/java/io/gnosis/data/backend/rpc/RpcClient.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,13 @@ class RpcClient(
}

private fun removeFee(tx: Transaction) {
when(tx) {
when (tx) {
is Transaction.Eip1559 -> {
tx.gas = null
tx.maxPriorityFee = null
tx.maxFeePerGas = null
}

is Transaction.Legacy -> {
tx.gas = null
tx.gasPrice = null
Expand All @@ -212,18 +213,20 @@ class RpcClient(
val balanceRequest = EthBalance(address = tx.from!!, id = 2)
val nonceRequest = EthGetTransactionCount(from = tx.from!!, id = 3)

val callRequest = when(tx) {
val callRequest = when (tx) {
is Transaction.Eip1559 -> {
EthCallEip1559(from = tx.from, transaction = tx, id = 4)
}

is Transaction.Legacy -> {
EthCall(from = tx.from, transaction = tx, id = 4)
}
}
val estimateRequest = when(tx) {
val estimateRequest = when (tx) {
is Transaction.Eip1559 -> {
EthEstimateGasEip1559(from = tx.from, transaction = tx, id = 5)
}

is Transaction.Legacy -> {
EthEstimateGas(from = tx.from, transaction = tx, id = 5)
}
Expand Down Expand Up @@ -261,6 +264,7 @@ class RpcClient(
is Transaction.Eip1559 -> {
byteArrayOf(tx.type, *tx.rlp(signature))
}

is Transaction.Legacy -> {
tx.rlp(signature)
}
Expand Down
3 changes: 2 additions & 1 deletion data/src/main/java/io/gnosis/data/models/Chain.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ data class Chain(
const val COL_FEATURES = "features"

val ID_MAINNET = BigInteger.valueOf(1)
val ID_RINKEBY = BigInteger.valueOf(4)
val ID_GOERLI = BigInteger.valueOf(5)
val ID_GNOSIS = BigInteger.valueOf(100)

val DEFAULT_CHAIN = Chain(
BuildConfig.CHAIN_ID.toBigInteger(),
Expand Down
2 changes: 1 addition & 1 deletion data/src/main/java/io/gnosis/data/repositories/UnstoppableDomainsRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class UnstoppableDomainsRepository(private val resolution: DomainResolution = Du
fun canResolve(chain: Chain): Boolean = providesDomainResolutionLibrary(chain.chainId) != null

private fun providesDomainResolutionLibrary(chainId: BigInteger): DomainResolution? {
if (chainId != Chain.ID_MAINNET && chainId != Chain.ID_RINKEBY) {
if (chainId != Chain.ID_MAINNET && chainId != Chain.ID_GOERLI) {
return null
}
return if (resolution is DummyDomainResolution) {
Expand Down
14 changes: 7 additions & 7 deletions data/src/test/java/io/gnosis/data/repositories/ChainInfoRepositoryTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ class ChainInfoRepositoryTest {
private val chainInfoRepository = ChainInfoRepository(chainDao, gatewayApi)

private val rinkebyChainInfo = ChainInfo(
Chain.ID_RINKEBY,
Chain.ID_GOERLI,
true,
"Rinkeby",
"rin",
"Goerli",
"gor",
null,
RpcUri(RpcAuthentication.API_KEY_PATH, ""),
BlockExplorerTemplate("", ""),
Expand Down Expand Up @@ -83,17 +83,17 @@ class ChainInfoRepositoryTest {
fun updateChainInfo() = runBlocking {
coEvery { chainDao.save(any()) } just Runs
coEvery { chainDao.saveCurrency(any()) } just Runs
val safes = listOf(Safe("0x00".asEthereumAddress()!!, "", Chain.ID_RINKEBY))
val safes = listOf(Safe("0x00".asEthereumAddress()!!, "", Chain.ID_GOERLI))

chainInfoRepository.updateChainInfo(pagedResult, safes)

coVerify(exactly = 1) {
chainDao.save(
Chain(
Chain.ID_RINKEBY,
Chain.ID_GOERLI,
true,
"Rinkeby",
"rin",
"Goerli",
"gor",
"",
"",
"",
Expand Down
12 changes: 6 additions & 6 deletions data/src/test/java/io/gnosis/data/repositories/UnstoppableRepositoryTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class UnstoppableRepositoryTest {


val addr = runBlocking {
repository.resolve(SUCCESS_DOMAIN, Chain.ID_RINKEBY)
repository.resolve(SUCCESS_DOMAIN, Chain.ID_GOERLI)
}

coVerify { resolutionLib.getAddress(SUCCESS_DOMAIN, "eth") }
Expand All @@ -54,7 +54,7 @@ class UnstoppableRepositoryTest {
)

try {
repository.resolve(FAIL_DOMAIN, Chain.ID_RINKEBY)
repository.resolve(FAIL_DOMAIN, Chain.ID_GOERLI)
} catch (err: NamingServiceException) {
assertTrue(err.code == NSExceptionCode.UnregisteredDomain)
}
Expand Down Expand Up @@ -85,15 +85,15 @@ class UnstoppableRepositoryTest {
}

@Test
fun `canResolve - (4) should succeed for Rinkeby`() {
fun `canResolve - (5) should succeed for Goerli`() {
repository = UnstoppableDomainsRepository()

val result = repository.canResolve(
Chain(
Chain.ID_RINKEBY,
Chain.ID_GOERLI,
true,
"Rinkeby",
"rin",
"Goerli",
"gor",
"",
"",
"",
Expand Down
Loading