Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/master' into frontend
Browse files Browse the repository at this point in the history
To mainly bring the ETH-crash bugfix and the amount vs minerFee
comparison sanity check (at broadcast time).
  • Loading branch information
knocte committed Oct 13, 2023
2 parents 79fdee7 + 6d4001d commit 56f2a30
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 36 deletions.
18 changes: 10 additions & 8 deletions src/GWallet.Backend/Account.fs
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,15 @@ module Account =
// FIXME: broadcasting shouldn't just get N consistent replies from FaultTolerantClient,
// but send it to as many as possible, otherwise it could happen that some server doesn't
// broadcast it even if you sent it
let BroadcastTransaction (trans: SignedTransaction<_>): Async<Uri> =
let BroadcastTransaction (trans: SignedTransaction<_>) (ignoreHigherMinerFeeThanAmount: bool): Async<Uri> =
async {
let currency = trans.TransactionInfo.Proposal.Amount.Currency

let! txId =
if currency.IsEtherBased() then
Ether.Account.BroadcastTransaction trans
Ether.Account.BroadcastTransaction trans ignoreHigherMinerFeeThanAmount
elif currency.IsUtxo() then
UtxoCoin.Account.BroadcastTransaction currency trans
UtxoCoin.Account.BroadcastTransaction currency trans ignoreHigherMinerFeeThanAmount
else
failwith <| SPrintF1 "Unknown currency %A" currency

Expand Down Expand Up @@ -371,14 +371,15 @@ module Account =
let SweepArchivedFunds (account: ArchivedAccount)
(balance: decimal)
(destination: IAccount)
(txMetadata: IBlockchainFeeInfo) =
(txMetadata: IBlockchainFeeInfo)
(ignoreHigherMinerFeeThanAmount: bool) =
match txMetadata with
| :? Ether.TransactionMetadata as etherTxMetadata ->
Ether.Account.SweepArchivedFunds account balance destination etherTxMetadata
Ether.Account.SweepArchivedFunds account balance destination etherTxMetadata ignoreHigherMinerFeeThanAmount
| :? UtxoCoin.TransactionMetadata as utxoTxMetadata ->
match account with
| :? UtxoCoin.ArchivedUtxoAccount as utxoAccount ->
UtxoCoin.Account.SweepArchivedFunds utxoAccount balance destination utxoTxMetadata
UtxoCoin.Account.SweepArchivedFunds utxoAccount balance destination utxoTxMetadata ignoreHigherMinerFeeThanAmount
| _ ->
failwith "If tx metadata is UTXO type, archived account should be too"
| _ -> failwith "tx metadata type unknown"
Expand All @@ -392,6 +393,7 @@ module Account =
(destination: string)
(amount: TransferAmount)
(password: string)
(ignoreHigherMinerFeeThanAmount: bool)
: Async<Uri> =
let baseAccount = account :> IAccount
if (baseAccount.PublicAddress.Equals(destination, StringComparison.InvariantCultureIgnoreCase)) then
Expand All @@ -410,14 +412,14 @@ module Account =
currency
match account with
| :? UtxoCoin.NormalUtxoAccount as utxoAccount ->
UtxoCoin.Account.SendPayment utxoAccount btcTxMetadata destination amount password
UtxoCoin.Account.SendPayment utxoAccount btcTxMetadata destination amount password ignoreHigherMinerFeeThanAmount
| _ ->
failwith "Account not Utxo-type but tx metadata is? report this bug (sendpayment)"

| :? Ether.TransactionMetadata as etherTxMetadata ->
if not (currency.IsEtherBased()) then
failwith "Account not ether-type but tx metadata is? report this bug (sendpayment)"
Ether.Account.SendPayment account etherTxMetadata destination amount password
Ether.Account.SendPayment account etherTxMetadata destination amount password ignoreHigherMinerFeeThanAmount
| _ ->
failwith "Unknown tx metadata type"

Expand Down
42 changes: 35 additions & 7 deletions src/GWallet.Backend/Ether/EtherAccount.fs
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,16 @@ module internal Account =
initialEthMinerFee

let feeValue = maybeBetterFee.CalculateAbsoluteValue()
if (amount.ValueToSend <> amount.BalanceAtTheMomentOfSending &&
feeValue > (amount.BalanceAtTheMomentOfSending - amount.ValueToSend)) then

let isSweepAndBalanceIsLessThanFee =
amount.ValueToSend = amount.BalanceAtTheMomentOfSending &&
amount.BalanceAtTheMomentOfSending < feeValue

let isNotSweepAndBalanceIsNotSufficient =
amount.ValueToSend <> amount.BalanceAtTheMomentOfSending &&
feeValue > amount.BalanceAtTheMomentOfSending - amount.ValueToSend

if isSweepAndBalanceIsLessThanFee || isNotSweepAndBalanceIsNotSufficient then
raise <| InsufficientBalanceForFee (Some feeValue)

return { Ether.Fee = maybeBetterFee; Ether.TransactionCount = txCount }
Expand Down Expand Up @@ -225,7 +233,25 @@ module internal Account =
return failwith <| SPrintF1 "Assertion failed: currency %A should be Ether or Ether token" account.Currency
}

let private BroadcastRawTransaction (currency: Currency) trans =
let private ValidateMinerFee (trans: string) =
let intDecoder = IntTypeDecoder()

let tx = TransactionFactory.CreateTransaction trans

let amountInWei = intDecoder.DecodeBigInteger tx.Value

// TODO: handle validating miner fee in token transfer (where amount is zero)
if amountInWei <> BigInteger.Zero then
let gasLimitInWei = intDecoder.DecodeBigInteger tx.GasLimit
let gasPriceInWei = intDecoder.DecodeBigInteger tx.GasPrice
let minerFeeInWei = gasLimitInWei * gasPriceInWei

if minerFeeInWei > amountInWei then
raise MinerFeeHigherThanOutputs

let private BroadcastRawTransaction (currency: Currency) trans (ignoreHigherMinerFeeThanAmount: bool) =
if not ignoreHigherMinerFeeThanAmount then
ValidateMinerFee trans
Ether.Server.BroadcastTransaction currency ("0x" + trans)

let BroadcastTransaction (trans: SignedTransaction<_>) =
Expand Down Expand Up @@ -360,19 +386,21 @@ module internal Account =
let SweepArchivedFunds (account: ArchivedAccount)
(balance: decimal)
(destination: IAccount)
(txMetadata: TransactionMetadata) =
(txMetadata: TransactionMetadata)
(ignoreHigherMinerFeeThanAmount: bool) =
let accountFrom = (account:>IAccount)
let amount = TransferAmount(balance, balance, accountFrom.Currency)
let ecPrivKey = EthECKey(account.GetUnencryptedPrivateKey())
let signedTrans = SignTransactionWithPrivateKey
account txMetadata destination.PublicAddress amount ecPrivKey
BroadcastRawTransaction accountFrom.Currency signedTrans
BroadcastRawTransaction accountFrom.Currency signedTrans ignoreHigherMinerFeeThanAmount

let SendPayment (account: NormalAccount)
(txMetadata: TransactionMetadata)
(destination: string)
(amount: TransferAmount)
(password: string) =
(password: string)
(ignoreHigherMinerFeeThanAmount: bool) =
let baseAccount = account :> IAccount
if (baseAccount.PublicAddress.Equals(destination, StringComparison.InvariantCultureIgnoreCase)) then
raise DestinationEqualToOrigin
Expand All @@ -381,7 +409,7 @@ module internal Account =

let trans = SignTransaction account txMetadata destination amount password

BroadcastRawTransaction currency trans
BroadcastRawTransaction currency trans ignoreHigherMinerFeeThanAmount

let private CreateInternal (password: string) (seed: array<byte>): FileRepresentation =
let privateKey = EthECKey(seed, true)
Expand Down
1 change: 1 addition & 0 deletions src/GWallet.Backend/Exceptions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ exception InvalidJson
exception TransactionAlreadySigned
exception TransactionNotSignedYet

exception MinerFeeHigherThanOutputs
46 changes: 41 additions & 5 deletions src/GWallet.Backend/UtxoCoin/UtxoCoinAccount.fs
Original file line number Diff line number Diff line change
Expand Up @@ -429,9 +429,43 @@ module Account =
let internal CheckValidPassword (account: NormalAccount) (password: string) =
GetPrivateKey account password |> ignore

let private BroadcastRawTransaction currency (rawTx: string): Async<string> =
let job = ElectrumClient.BroadcastTransaction rawTx
Server.Query currency QuerySettings.Broadcast job None
let private ValidateMinerFee currency (rawTransaction: string) =
async {
let network = GetNetwork currency

let txToValidate = Transaction.Parse (rawTransaction, network)

let totalOutputsAmount = txToValidate.TotalOut

let getInputAmount (input: TxIn) =
async {
let job = ElectrumClient.GetBlockchainTransaction (input.PrevOut.Hash.ToString())
let! inputOriginTxString = Server.Query currency (QuerySettings.Default ServerSelectionMode.Fast) job None
let inputOriginTx = Transaction.Parse (inputOriginTxString, network)
return inputOriginTx.Outputs.[input.PrevOut.N].Value
}

let! amounts =
txToValidate.Inputs
|> Seq.map getInputAmount
|> Async.Parallel

let totalInputsAmount = Seq.sum amounts

let minerFee = totalInputsAmount - totalOutputsAmount
if minerFee > totalOutputsAmount then
return raise MinerFeeHigherThanOutputs

return ()
}

let private BroadcastRawTransaction currency (rawTx: string) (ignoreHigherMinerFeeThanAmount: bool): Async<string> =
async {
if not ignoreHigherMinerFeeThanAmount then
do! ValidateMinerFee currency rawTx
let job = ElectrumClient.BroadcastTransaction rawTx
return! Server.Query currency QuerySettings.Broadcast job None
}

let internal BroadcastTransaction currency (transaction: SignedTransaction<_>) =
// FIXME: stop embedding TransactionInfo element in SignedTransaction<BTC>
Expand All @@ -443,13 +477,14 @@ module Account =
(destination: string)
(amount: TransferAmount)
(password: string)
(ignoreHigherMinerFeeThanAmount: bool)
=
let baseAccount = account :> IAccount
if (baseAccount.PublicAddress.Equals(destination, StringComparison.InvariantCultureIgnoreCase)) then
raise DestinationEqualToOrigin

let finalTransaction = SignTransaction account txMetadata destination amount password
BroadcastRawTransaction baseAccount.Currency finalTransaction
BroadcastRawTransaction baseAccount.Currency finalTransaction ignoreHigherMinerFeeThanAmount

// TODO: maybe move this func to Backend.Account module, or simply inline it (simple enough)
let public ExportUnsignedTransactionToJson trans =
Expand All @@ -472,14 +507,15 @@ module Account =
(balance: decimal)
(destination: IAccount)
(txMetadata: TransactionMetadata)
(ignoreHigherMinerFeeThanAmount: bool)
=
let currency = (account:>IAccount).Currency
let network = GetNetwork currency
let amount = TransferAmount(balance, balance, currency)
let privateKey = Key.Parse(account.GetUnencryptedPrivateKey(), network)
let signedTrans = SignTransactionWithPrivateKey
account txMetadata destination.PublicAddress amount privateKey
BroadcastRawTransaction currency (signedTrans.ToHex())
BroadcastRawTransaction currency (signedTrans.ToHex()) ignoreHigherMinerFeeThanAmount

let internal Create currency (password: string) (seed: array<byte>): Async<FileRepresentation> =
async {
Expand Down
47 changes: 34 additions & 13 deletions src/GWallet.Frontend.Console/Program.fs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,31 @@ open System.Text.RegularExpressions
open GWallet.Backend
open GWallet.Frontend.Console

let SendWithMinerFeeValidation (sendPaymentFunc: bool -> unit) =
try
sendPaymentFunc false

UserInteraction.PressAnyKeyToContinue ()
with
| :? MinerFeeHigherThanOutputs ->
let userWantsToContinue =
UserInteraction.AskYesNo "Miner fee is higher than amount transferred, are you ABSOLUTELY sure you want to broadcast?"
if userWantsToContinue then
sendPaymentFunc true

UserInteraction.PressAnyKeyToContinue ()

let rec TrySendAmount (account: NormalAccount) transactionMetadata destination amount =
let password = UserInteraction.AskPassword false
try

let sendPayment ignoreHigherMinerFeeThanAmount =
let txIdUri =
Account.SendPayment account transactionMetadata destination amount password
Account.SendPayment account transactionMetadata destination amount password ignoreHigherMinerFeeThanAmount
|> Async.RunSynchronously
Console.WriteLine(sprintf "Transaction successful:%s%s" Environment.NewLine (txIdUri.ToString()))
UserInteraction.PressAnyKeyToContinue ()

try
SendWithMinerFeeValidation sendPayment
with
| :? DestinationEqualToOrigin ->
Presentation.Error "Transaction's origin cannot be the same as the destination."
Expand Down Expand Up @@ -69,11 +86,13 @@ let BroadcastPayment() =

if UserInteraction.AskYesNo "Do you accept?" then
try
let txIdUri =
Account.BroadcastTransaction signedTransaction
|> Async.RunSynchronously
Console.WriteLine(sprintf "Transaction successful:%s%s" Environment.NewLine (txIdUri.ToString()))
UserInteraction.PressAnyKeyToContinue ()
let broadcastTx ignoreHigherMinerFeeThanAmount =
let txIdUri =
Account.BroadcastTransaction signedTransaction ignoreHigherMinerFeeThanAmount
|> Async.RunSynchronously
Console.WriteLine(sprintf "Transaction successful:%s%s" Environment.NewLine (txIdUri.ToString()))

SendWithMinerFeeValidation broadcastTx
with
| :? DestinationEqualToOrigin ->
Presentation.Error "Transaction's origin cannot be the same as the destination."
Expand Down Expand Up @@ -412,11 +431,13 @@ let rec CheckArchivedAccountsAreEmpty(): bool =
match maybeFee with
| None -> ()
| Some(feeInfo) ->
let txId =
Account.SweepArchivedFunds archivedAccount balance account feeInfo
|> Async.RunSynchronously
Console.WriteLine(sprintf "Transaction successful, its ID is:%s%s" Environment.NewLine txId)
UserInteraction.PressAnyKeyToContinue ()
let sweepFunds ignoreHigherMinerFeeThanAmount =
let txId =
Account.SweepArchivedFunds archivedAccount balance account feeInfo ignoreHigherMinerFeeThanAmount
|> Async.RunSynchronously
Console.WriteLine(sprintf "Transaction successful, its ID is:%s%s" Environment.NewLine txId)

SendWithMinerFeeValidation sweepFunds

not (archivedAccountsInNeedOfAction.Any())

Expand Down
6 changes: 3 additions & 3 deletions src/GWallet.Frontend.Console/UserInteraction.fs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ module UserInteraction =
| :? ReadOnlyAccount -> "(READ-ONLY)"
| _ -> String.Empty

let accountInfo = sprintf "Account %d: %s%sCurrency=[%A] Address=[%s]"
let accountInfo = sprintf "Account %d: %s%sCurrency=[ %A ] Address=[ %s ]"
accountNumber maybeReadOnly Environment.NewLine
account.Currency
account.PublicAddress
Expand All @@ -209,13 +209,13 @@ module UserInteraction =
| NotFresh(NotAvailable) ->
yield "Unknown balance (Network unreachable... off-line?)"
| NotFresh(Cached(balance,time)) ->
let status = sprintf "Last known balance=[%s] (as of %s) %s"
let status = sprintf "Last known balance=[ %s ] (as of %s) %s"
(balance |> Formatting.DecimalAmountRounding CurrencyType.Crypto)
(time |> Formatting.ShowSaneDate)
(BalanceInUsdString balance maybeUsdValue)
yield status
| Fresh(balance) ->
let status = sprintf "Balance=[%s] %s"
let status = sprintf "Balance=[ %s ] %s"
(balance |> Formatting.DecimalAmountRounding CurrencyType.Crypto)
(BalanceInUsdString balance maybeUsdValue)
yield status
Expand Down

0 comments on commit 56f2a30

Please sign in to comment.