From ca9820b0652a7eff8757207d9de9ffe75a4de150 Mon Sep 17 00:00:00 2001 From: b00f Date: Sun, 10 Nov 2024 15:29:48 +0800 Subject: [PATCH] fix(txpool, cmd, gtk): broadcast transactions with zero fee (#1589) Co-authored-by: Javad Rajabzadeh --- cmd/gtk/assets/ui/dialog_transaction_bond.ui | 2 +- .../assets/ui/dialog_transaction_transfer.ui | 2 +- .../assets/ui/dialog_transaction_withdraw.ui | 2 +- cmd/gtk/dialog_address_change_password.go | 8 +- cmd/gtk/dialog_addresss_private_key.go | 6 +- cmd/gtk/dialog_transaction_bond.go | 23 +- cmd/gtk/dialog_transaction_transfer.go | 24 +- cmd/gtk/dialog_transaction_unbond.go | 2 +- cmd/gtk/dialog_transaction_withdraw.go | 23 +- cmd/gtk/dialog_wallet_create_address.go | 8 +- cmd/gtk/model_wallet.go | 4 +- cmd/gtk/startup_assistant.go | 6 +- cmd/gtk/utils.go | 50 +- cmd/gtk/widget_wallet.go | 6 +- cmd/wallet/tx.go | 25 +- consensus/consensus_test.go | 2 +- execution/errors.go | 13 +- execution/execution.go | 18 - execution/execution_test.go | 51 -- sandbox/sandbox_test.go | 4 +- tests/transaction_test.go | 4 - txpool/config.go | 8 +- txpool/config_test.go | 11 +- txpool/errors.go | 16 +- txpool/txpool.go | 131 +++--- txpool/txpool_test.go | 438 +++++++++++------- types/block/block_test.go | 2 +- types/tx/tx.go | 33 +- types/tx/tx_test.go | 76 ++- util/testsuite/testsuite.go | 16 +- wallet/tx_builder.go | 64 ++- wallet/wallet.go | 14 +- www/grpc/server_test.go | 2 +- 33 files changed, 612 insertions(+), 482 deletions(-) diff --git a/cmd/gtk/assets/ui/dialog_transaction_bond.ui b/cmd/gtk/assets/ui/dialog_transaction_bond.ui index 876db3a7a..9ba6bdc23 100644 --- a/cmd/gtk/assets/ui/dialog_transaction_bond.ui +++ b/cmd/gtk/assets/ui/dialog_transaction_bond.ui @@ -272,7 +272,7 @@ True False start - Fee (Optional): + Fee: False diff --git a/cmd/gtk/assets/ui/dialog_transaction_transfer.ui b/cmd/gtk/assets/ui/dialog_transaction_transfer.ui index bbde1bab6..6337c292b 100644 --- a/cmd/gtk/assets/ui/dialog_transaction_transfer.ui +++ b/cmd/gtk/assets/ui/dialog_transaction_transfer.ui @@ -231,7 +231,7 @@ True False start - Fee (Optional): + Fee: False diff --git a/cmd/gtk/assets/ui/dialog_transaction_withdraw.ui b/cmd/gtk/assets/ui/dialog_transaction_withdraw.ui index d08cd73cd..f8bfc9732 100644 --- a/cmd/gtk/assets/ui/dialog_transaction_withdraw.ui +++ b/cmd/gtk/assets/ui/dialog_transaction_withdraw.ui @@ -226,7 +226,7 @@ True False start - Fee (Optional): + Fee: False diff --git a/cmd/gtk/dialog_address_change_password.go b/cmd/gtk/dialog_address_change_password.go index dc8ba2b1b..67cff68a2 100644 --- a/cmd/gtk/dialog_address_change_password.go +++ b/cmd/gtk/dialog_address_change_password.go @@ -47,10 +47,14 @@ func changePassword(wlt *wallet.Wallet) { } err = wlt.UpdatePassword(oldPassword, newPassword) - errorCheck(err) + if err != nil { + showError(err) + + return + } err = wlt.Save() - errorCheck(err) + fatalErrorCheck(err) dlg.Close() } diff --git a/cmd/gtk/dialog_addresss_private_key.go b/cmd/gtk/dialog_addresss_private_key.go index 302aa3cdf..90e4500ec 100644 --- a/cmd/gtk/dialog_addresss_private_key.go +++ b/cmd/gtk/dialog_addresss_private_key.go @@ -22,7 +22,11 @@ func showAddressPrivateKey(wlt *wallet.Wallet, addr string) { } prv, err := wlt.PrivateKey(password, addr) - errorCheck(err) + if err != nil { + showError(err) + + return + } dlg := getDialogObj(builder, "id_dialog_address_private_key") addressEntry := getEntryObj(builder, "id_entry_address") diff --git a/cmd/gtk/dialog_transaction_bond.go b/cmd/gtk/dialog_transaction_bond.go index bb18f7540..932c5cbf9 100644 --- a/cmd/gtk/dialog_transaction_bond.go +++ b/cmd/gtk/dialog_transaction_bond.go @@ -7,7 +7,6 @@ import ( "fmt" "github.com/gotk3/gotk3/gtk" - "github.com/pactus-project/pactus/cmd" "github.com/pactus-project/pactus/types/amount" "github.com/pactus-project/pactus/types/tx/payload" "github.com/pactus-project/pactus/wallet" @@ -35,6 +34,9 @@ func broadcastTransactionBond(wlt *wallet.Wallet) { getButtonObj(builder, "id_button_cancel").SetImage(CancelIcon()) getButtonObj(builder, "id_button_send").SetImage(SendIcon()) + estimatedFee := estimatedFee(wlt, payload.TypeBond) + feeEntry.SetText(fmt.Sprintf("%g", estimatedFee.ToPAC())) + for _, ai := range wlt.AllAccountAddresses() { senderEntry.Append(ai.Address, ai.Address) } @@ -48,6 +50,7 @@ func broadcastTransactionBond(wlt *wallet.Wallet) { onSenderChanged := func() { senderStr := senderEntry.GetActiveID() updateAccountHint(senderHint, senderStr, wlt) + updateBalanceHint(amountHint, senderEntry.GetActiveID(), wlt) } onReceiverChanged := func() { @@ -56,10 +59,6 @@ func broadcastTransactionBond(wlt *wallet.Wallet) { updateValidatorHint(receiverHint, receiverStr, wlt) } - onAmountChanged := func() { - updateAmountHint(amountHint, senderEntry.GetActiveID(), wlt) - } - onFeeChanged := func() { updateFeeHint(feeHint, wlt, payload.TypeTransfer) } @@ -74,25 +73,20 @@ func broadcastTransactionBond(wlt *wallet.Wallet) { amt, err := amount.FromString(amountStr) if err != nil { - errorCheck(err) + showError(err) return } + feeStr, _ := feeEntry.GetText() opts := []wallet.TxOption{ wallet.OptionMemo(memo), - } - - fee, _ := feeEntry.GetText() - if fee != "" { - feeAmount, err := amount.FromString(fee) - cmd.FatalErrorCheck(err) - opts = append(opts, wallet.OptionFee(feeAmount)) + wallet.OptionFeeFromString(feeStr), } trx, err := wlt.MakeBondTx(sender, receiver, publicKey, amt, opts...) if err != nil { - errorCheck(err) + showError(err) return } @@ -120,7 +114,6 @@ Memo: %s signals := map[string]any{ "on_sender_changed": onSenderChanged, "on_receiver_changed": onReceiverChanged, - "on_amount_changed": onAmountChanged, "on_fee_changed": onFeeChanged, "on_send": onSend, "on_cancel": onClose, diff --git a/cmd/gtk/dialog_transaction_transfer.go b/cmd/gtk/dialog_transaction_transfer.go index eed7d4ac3..db3a2de4a 100644 --- a/cmd/gtk/dialog_transaction_transfer.go +++ b/cmd/gtk/dialog_transaction_transfer.go @@ -7,7 +7,6 @@ import ( "fmt" "github.com/gotk3/gotk3/gtk" - "github.com/pactus-project/pactus/cmd" "github.com/pactus-project/pactus/types/amount" "github.com/pactus-project/pactus/types/tx/payload" "github.com/pactus-project/pactus/wallet" @@ -34,6 +33,9 @@ func broadcastTransactionTransfer(wlt *wallet.Wallet) { getButtonObj(builder, "id_button_cancel").SetImage(CancelIcon()) getButtonObj(builder, "id_button_send").SetImage(SendIcon()) + estimatedFee := estimatedFee(wlt, payload.TypeTransfer) + feeEntry.SetText(fmt.Sprintf("%g", estimatedFee.ToPAC())) + for _, i := range wlt.AllAccountAddresses() { senderEntry.Append(i.Address, i.Address) } @@ -42,17 +44,13 @@ func broadcastTransactionTransfer(wlt *wallet.Wallet) { onSenderChanged := func() { senderStr := senderEntry.GetActiveID() updateAccountHint(senderHint, senderStr, wlt) + updateBalanceHint(amountHint, senderEntry.GetActiveID(), wlt) } onReceiverChanged := func() { receiverStr, _ := receiverEntry.GetText() updateAccountHint(receiverHint, receiverStr, wlt) } - - onAmountChanged := func() { - updateAmountHint(amountHint, senderEntry.GetActiveID(), wlt) - } - onFeeChanged := func() { updateFeeHint(feeHint, wlt, payload.TypeTransfer) } @@ -65,25 +63,20 @@ func broadcastTransactionTransfer(wlt *wallet.Wallet) { amt, err := amount.FromString(amountStr) if err != nil { - errorCheck(err) + showError(err) return } + feeStr, _ := feeEntry.GetText() opts := []wallet.TxOption{ wallet.OptionMemo(memo), - } - - fee, _ := feeEntry.GetText() - if fee != "" { - feeAmount, err := amount.FromString(fee) - cmd.FatalErrorCheck(err) - opts = append(opts, wallet.OptionFee(feeAmount)) + wallet.OptionFeeFromString(feeStr), } trx, err := wlt.MakeTransferTx(sender, receiver, amt, opts...) if err != nil { - errorCheck(err) + showError(err) return } @@ -111,7 +104,6 @@ Memo: %s signals := map[string]any{ "on_sender_changed": onSenderChanged, "on_receiver_changed": onReceiverChanged, - "on_amount_changed": onAmountChanged, "on_fee_changed": onFeeChanged, "on_send": onSend, "on_cancel": onClose, diff --git a/cmd/gtk/dialog_transaction_unbond.go b/cmd/gtk/dialog_transaction_unbond.go index 7966dae07..b2a65ea67 100644 --- a/cmd/gtk/dialog_transaction_unbond.go +++ b/cmd/gtk/dialog_transaction_unbond.go @@ -46,7 +46,7 @@ func broadcastTransactionUnbond(wlt *wallet.Wallet) { trx, err := wlt.MakeUnbondTx(validator, opts...) if err != nil { - errorCheck(err) + showError(err) return } diff --git a/cmd/gtk/dialog_transaction_withdraw.go b/cmd/gtk/dialog_transaction_withdraw.go index 28290628c..57a8a8f0e 100644 --- a/cmd/gtk/dialog_transaction_withdraw.go +++ b/cmd/gtk/dialog_transaction_withdraw.go @@ -7,7 +7,6 @@ import ( "fmt" "github.com/gotk3/gotk3/gtk" - "github.com/pactus-project/pactus/cmd" "github.com/pactus-project/pactus/types/amount" "github.com/pactus-project/pactus/types/tx/payload" "github.com/pactus-project/pactus/wallet" @@ -34,6 +33,9 @@ func broadcastTransactionWithdraw(wlt *wallet.Wallet) { getButtonObj(builder, "id_button_cancel").SetImage(CancelIcon()) getButtonObj(builder, "id_button_send").SetImage(SendIcon()) + estimatedFee := estimatedFee(wlt, payload.TypeWithdraw) + feeEntry.SetText(fmt.Sprintf("%g", estimatedFee.ToPAC())) + for _, ai := range wlt.AllValidatorAddresses() { validatorEntry.Append(ai.Address, ai.Address) } @@ -46,6 +48,7 @@ func broadcastTransactionWithdraw(wlt *wallet.Wallet) { onSenderChanged := func() { senderStr := validatorEntry.GetActiveID() updateValidatorHint(validatorHint, senderStr, wlt) + updateStakeHint(stakeHint, validatorEntry.GetActiveID(), wlt) } onReceiverChanged := func() { @@ -54,10 +57,6 @@ func broadcastTransactionWithdraw(wlt *wallet.Wallet) { updateAccountHint(receiverHint, receiverStr, wlt) } - onAmountChanged := func() { - updateAmountHint(stakeHint, validatorEntry.GetActiveID(), wlt) - } - onFeeChanged := func() { updateFeeHint(feeHint, wlt, payload.TypeTransfer) } @@ -71,25 +70,20 @@ func broadcastTransactionWithdraw(wlt *wallet.Wallet) { amt, err := amount.FromString(amountStr) if err != nil { - errorCheck(err) + showError(err) return } + feeStr, _ := feeEntry.GetText() opts := []wallet.TxOption{ wallet.OptionMemo(memo), - } - - fee, _ := feeEntry.GetText() - if fee != "" { - feeAmount, err := amount.FromString(fee) - cmd.FatalErrorCheck(err) - opts = append(opts, wallet.OptionFee(feeAmount)) + wallet.OptionFeeFromString(feeStr), } trx, err := wlt.MakeWithdrawTx(sender, receiver, amt, opts...) if err != nil { - errorCheck(err) + showError(err) return } @@ -117,7 +111,6 @@ Memo: %s signals := map[string]any{ "on_sender_changed": onSenderChanged, "on_receiver_changed": onReceiverChanged, - "on_stake_changed": onAmountChanged, "on_fee_changed": onFeeChanged, "on_send": onSend, "on_cancel": onClose, diff --git a/cmd/gtk/dialog_wallet_create_address.go b/cmd/gtk/dialog_wallet_create_address.go index 7d22e4e72..2e74344d2 100644 --- a/cmd/gtk/dialog_wallet_create_address.go +++ b/cmd/gtk/dialog_wallet_create_address.go @@ -50,10 +50,14 @@ func createAddress(wdgWallet *widgetWallet) { _, err = wdgWallet.model.wallet.NewValidatorAddress(walletAddressLabel) } - errorCheck(err) + if err != nil { + showError(err) + + return + } err = wdgWallet.model.wallet.Save() - errorCheck(err) + fatalErrorCheck(err) wdgWallet.model.rebuildModel() diff --git a/cmd/gtk/model_wallet.go b/cmd/gtk/model_wallet.go index d52625a56..44314f296 100644 --- a/cmd/gtk/model_wallet.go +++ b/cmd/gtk/model_wallet.go @@ -77,7 +77,7 @@ func (model *walletModel) rebuildModel() { model.listStore.Clear() for _, item := range data { iter := model.listStore.Append() - err := model.listStore.Set(iter, + _ = model.listStore.Set(iter, []int{ IDAddressesColumnNo, IDAddressesColumnAddress, @@ -94,8 +94,6 @@ func (model *walletModel) rebuildModel() { item[4], item[5], }) - - errorCheck(err) } return false diff --git a/cmd/gtk/startup_assistant.go b/cmd/gtk/startup_assistant.go index 4d80ebf82..1bd3f29da 100644 --- a/cmd/gtk/startup_assistant.go +++ b/cmd/gtk/startup_assistant.go @@ -225,7 +225,11 @@ func startupAssistant(workingDir string, chainType genesis.ChainType) bool { snapshotURL, storeDir, ) - fatalErrorCheck(err) + if err != nil { + showError(err) + + return + } ctx := context.Background() mdCh := getMetadata(ctx, importer, listBox) diff --git a/cmd/gtk/utils.go b/cmd/gtk/utils.go index f45f67789..d8829e757 100644 --- a/cmd/gtk/utils.go +++ b/cmd/gtk/utils.go @@ -13,6 +13,7 @@ import ( "github.com/gotk3/gotk3/gdk" "github.com/gotk3/gotk3/glib" "github.com/gotk3/gotk3/gtk" + "github.com/pactus-project/pactus/types/amount" "github.com/pactus-project/pactus/types/tx" "github.com/pactus-project/pactus/types/tx/payload" "github.com/pactus-project/pactus/wallet" @@ -67,13 +68,14 @@ func showErrorDialog(parent gtk.IWindow, msg string) { dlg.Destroy() } -func errorCheck(err error) { - if err != nil { - showErrorDialog(nil, err.Error()) - log.Print(err.Error()) - } +// showError displays an error dialog and logs the error message. +func showError(err error) { + showErrorDialog(nil, err.Error()) + log.Print(err.Error()) } +// fatalErrorCheck checks for an error, shows an error dialog and terminates the program. +// Use with caution. func fatalErrorCheck(err error) { if err != nil { showErrorDialog(nil, err.Error()) @@ -187,18 +189,34 @@ func updateAccountHint(lbl *gtk.Label, addr string, wlt *wallet.Wallet) { } } -func updateFeeHint(lbl *gtk.Label, wlt *wallet.Wallet, payloadType payload.Type) { +func updateFeeHint(_ *gtk.Label, _ *wallet.Wallet, _ payload.Type) { + // Nothing for now! + // The goal is to show an estimate of how long it takes for a transaction + // with the given fee to be confirmed (confirmation time). + // We can analyze data from past blocks to estimate the confirmation time. +} + +func estimatedFee(wlt *wallet.Wallet, payloadType payload.Type) amount.Amount { fee, _ := wlt.CalculateFee(0, payloadType) - updateHintLabel(lbl, fmt.Sprintf("fee: %s", fee)) + + return fee } -func updateAmountHint(lbl *gtk.Label, addr string, wlt *wallet.Wallet) { +func updateBalanceHint(lbl *gtk.Label, addr string, wlt *wallet.Wallet) { balance, err := wlt.Balance(addr) if err == nil { - updateHintLabel(lbl, fmt.Sprintf("balance: %s", balance)) + updateHintLabel(lbl, fmt.Sprintf("Total Balance: %s", balance)) } else { - stake, _ := wlt.Stake(addr) - updateHintLabel(lbl, fmt.Sprintf("stake: %s", stake)) + updateHintLabel(lbl, "") + } +} + +func updateStakeHint(lbl *gtk.Label, addr string, wlt *wallet.Wallet) { + stake, err := wlt.Stake(addr) + if err == nil { + updateHintLabel(lbl, fmt.Sprintf("Total Stake: %s", stake)) + } else { + updateHintLabel(lbl, "") } } @@ -215,23 +233,19 @@ func signAndBroadcastTransaction(parent *gtk.Dialog, msg string, wlt *wallet.Wal } err := wlt.SignTransaction(password, trx) if err != nil { - errorCheck(err) + showError(err) return } txID, err := wlt.BroadcastTransaction(trx) if err != nil { - errorCheck(err) + showError(err) return } err = wlt.Save() - if err != nil { - errorCheck(err) - - return - } + fatalErrorCheck(err) showInfoDialog(parent, fmt.Sprintf("Transaction Hash: %s", txID, txID)) diff --git a/cmd/gtk/widget_wallet.go b/cmd/gtk/widget_wallet.go index 3216ff1f2..aa0fa74c5 100644 --- a/cmd/gtk/widget_wallet.go +++ b/cmd/gtk/widget_wallet.go @@ -174,7 +174,11 @@ func (ww *widgetWallet) onShowSeed() { } seed, err := ww.model.wallet.Mnemonic(password) - errorCheck(err) + if err != nil { + showError(err) + + return + } showSeed(seed) } diff --git a/cmd/wallet/tx.go b/cmd/wallet/tx.go index 674206169..e2b426811 100644 --- a/cmd/wallet/tx.go +++ b/cmd/wallet/tx.go @@ -40,14 +40,11 @@ func buildTransferTxCmd(parentCmd *cobra.Command) { amt, err := amount.FromString(args[2]) cmd.FatalErrorCheck(err) - fee, err := amount.NewAmount(*feeOpt) - cmd.FatalErrorCheck(err) - wlt, err := openWallet() cmd.FatalErrorCheck(err) opts := []wallet.TxOption{ - wallet.OptionFee(fee), + wallet.OptionFeeFromString(*feeOpt), wallet.OptionLockTime(uint32(*lockTimeOpt)), wallet.OptionMemo(*memoOpt), } @@ -86,14 +83,11 @@ func buildBondTxCmd(parentCmd *cobra.Command) { amt, err := amount.FromString(args[2]) cmd.FatalErrorCheck(err) - fee, err := amount.NewAmount(*feeOpt) - cmd.FatalErrorCheck(err) - wlt, err := openWallet() cmd.FatalErrorCheck(err) opts := []wallet.TxOption{ - wallet.OptionFee(fee), + wallet.OptionFeeFromString(*feeOpt), wallet.OptionLockTime(uint32(*lockTime)), wallet.OptionMemo(*memoOpt), } @@ -128,14 +122,11 @@ func buildUnbondTxCmd(parentCmd *cobra.Command) { unbondCmd.Run = func(_ *cobra.Command, args []string) { from := args[0] - fee, err := amount.NewAmount(*feeOpt) - cmd.FatalErrorCheck(err) - wlt, err := openWallet() cmd.FatalErrorCheck(err) opts := []wallet.TxOption{ - wallet.OptionFee(fee), + wallet.OptionFeeFromString(*feeOpt), wallet.OptionLockTime(uint32(*lockTime)), wallet.OptionMemo(*memoOpt), } @@ -171,7 +162,7 @@ func buildWithdrawTxCmd(parentCmd *cobra.Command) { amt, err := amount.FromString(args[2]) cmd.FatalErrorCheck(err) - fee, err := amount.NewAmount(*feeOpt) + fee, err := amount.FromString(*feeOpt) cmd.FatalErrorCheck(err) wlt, err := openWallet() @@ -198,12 +189,12 @@ func buildWithdrawTxCmd(parentCmd *cobra.Command) { } } -func addCommonTxOptions(cobra *cobra.Command) (*int, *float64, *string, *bool) { +func addCommonTxOptions(cobra *cobra.Command) (*int, *string, *string, *bool) { lockTimeOpt := cobra.Flags().Int("lock-time", 0, - "transaction lock-time, if not specified will be the current height") + "transaction lock-time, if not specified will be the latest height") - feeOpt := cobra.Flags().Float64("fee", 0, - "transaction fee in PAC, if not specified will calculate from the amount") + feeOpt := cobra.Flags().String("fee", "", + "transaction fee in PAC, if not specified will set to the estimated value") memoOpt := cobra.Flags().String("memo", "", "transaction memo, maximum should be 64 character") diff --git a/consensus/consensus_test.go b/consensus/consensus_test.go index 940dc6e99..0c65f3369 100644 --- a/consensus/consensus_test.go +++ b/consensus/consensus_test.go @@ -71,7 +71,7 @@ func setupWithSeed(t *testing.T, seed int64) *testData { fmt.Printf("=== test %s, seed: %d\n", t.Name(), seed) - ts := testsuite.NewTestSuiteForSeed(seed) + ts := testsuite.NewTestSuiteFromSeed(seed) _, valKeys := ts.GenerateTestCommittee(4) txPool := txpool.MockingTxPool() diff --git a/execution/errors.go b/execution/errors.go index 79a5f850d..0640c8960 100644 --- a/execution/errors.go +++ b/execution/errors.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/pactus-project/pactus/crypto" - "github.com/pactus-project/pactus/types/amount" "github.com/pactus-project/pactus/types/tx" ) @@ -17,7 +16,7 @@ type TransactionCommittedError struct { } func (e TransactionCommittedError) Error() string { - return fmt.Sprintf("the transaction committed before: %s", + return fmt.Sprintf("the transaction submitted before: %s", e.ID.String()) } @@ -43,16 +42,6 @@ func (e LockTimeInFutureError) Error() string { return fmt.Sprintf("lock time is in the future: %v", e.LockTime) } -// InvalidFeeError is returned when the transaction fee is not valid. -type InvalidFeeError struct { - Fee amount.Amount - Expected amount.Amount -} - -func (e InvalidFeeError) Error() string { - return fmt.Sprintf("fee is invalid, expected: %s, got: %s", e.Expected, e.Fee) -} - // SignerBannedError is returned when the signer of transaction is banned and its assets is freezed. type SignerBannedError struct { addr crypto.Address diff --git a/execution/execution.go b/execution/execution.go index 312a87f3f..ac776ab77 100644 --- a/execution/execution.go +++ b/execution/execution.go @@ -40,10 +40,6 @@ func CheckAndExecute(trx *tx.Tx, sbx sandbox.Sandbox, strict bool) error { return err } - if err := CheckFee(trx); err != nil { - return err - } - if err := exe.Check(strict); err != nil { return err } @@ -84,17 +80,3 @@ func CheckLockTime(trx *tx.Tx, sbx sandbox.Sandbox, strict bool) error { return nil } - -func CheckFee(trx *tx.Tx) error { - // TODO: This check maybe can be done in BasicCheck? - if trx.IsFreeTx() { - if trx.Fee() != 0 { - return InvalidFeeError{ - Fee: trx.Fee(), - Expected: 0, - } - } - } - - return nil -} diff --git a/execution/execution_test.go b/execution/execution_test.go index 33a19d11e..84438787a 100644 --- a/execution/execution_test.go +++ b/execution/execution_test.go @@ -6,7 +6,6 @@ import ( "github.com/pactus-project/pactus/crypto" "github.com/pactus-project/pactus/execution/executor" "github.com/pactus-project/pactus/sandbox" - "github.com/pactus-project/pactus/types/amount" "github.com/pactus-project/pactus/types/tx" "github.com/pactus-project/pactus/util/testsuite" "github.com/stretchr/testify/assert" @@ -184,48 +183,6 @@ func TestSubsidyLockTime(t *testing.T) { } } -func TestCheckFee(t *testing.T) { - ts := testsuite.NewTestSuite(t) - - tests := []struct { - name string - trx *tx.Tx - expectedErr error - }{ - { - name: "Subsidy transaction with fee", - trx: tx.NewTransferTx(ts.RandHeight(), crypto.TreasuryAddress, ts.RandAccAddress(), - ts.RandAmount(), 1), - expectedErr: InvalidFeeError{Fee: 1, Expected: 0}, - }, - { - name: "Subsidy transaction without fee", - trx: tx.NewTransferTx(ts.RandHeight(), crypto.TreasuryAddress, ts.RandAccAddress(), - ts.RandAmount(), 0), - expectedErr: nil, - }, - { - name: "Transfer transaction with fee", - trx: tx.NewTransferTx(ts.RandHeight(), ts.RandAccAddress(), ts.RandAccAddress(), - ts.RandAmount(), 0), - expectedErr: nil, - }, - { - name: "Transfer transaction without fee", - trx: tx.NewTransferTx(ts.RandHeight(), ts.RandAccAddress(), ts.RandAccAddress(), - ts.RandAmount(), 0), - expectedErr: nil, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := CheckFee(tt.trx) - assert.ErrorIs(t, err, tt.expectedErr) - }) - } -} - func TestExecute(t *testing.T) { ts := testsuite.NewTestSuite(t) @@ -266,14 +223,6 @@ func TestCheck(t *testing.T) { assert.ErrorIs(t, err, LockTimeInFutureError{LockTime: invalidLocoTme}) }) - t.Run("Invalid fee, Should return error", func(t *testing.T) { - invalidFee := amount.Amount(1) - trx := tx.NewTransferTx(lockTime, crypto.TreasuryAddress, ts.RandAccAddress(), ts.RandAmount(), invalidFee) - - err := CheckAndExecute(trx, sbx, true) - assert.ErrorIs(t, err, InvalidFeeError{Fee: invalidFee, Expected: 0}) - }) - t.Run("Invalid transaction, Should return error", func(t *testing.T) { randAddr := ts.RandAccAddress() trx := tx.NewTransferTx(lockTime, randAddr, ts.RandAccAddress(), ts.RandAmount(), ts.RandFee()) diff --git a/sandbox/sandbox_test.go b/sandbox/sandbox_test.go index 8f979175f..3b2b08a45 100644 --- a/sandbox/sandbox_test.go +++ b/sandbox/sandbox_test.go @@ -278,14 +278,14 @@ func TestUpdateFromOutsideTheSandbox(t *testing.T) { t.Run("Try update an account from outside the sandbox, Should panic", func(t *testing.T) { assert.Panics(t, func() { - acc, addr := td.GenerateTestAccount(td.RandInt32(0)) + acc, addr := td.GenerateTestAccount(td.RandInt32(10000)) td.sbx.UpdateAccount(addr, acc) }) }) t.Run("Try update a validator from outside the sandbox, Should panic", func(t *testing.T) { assert.Panics(t, func() { - val, _ := td.GenerateTestValidator(td.RandInt32(0)) + val, _ := td.GenerateTestValidator(td.RandInt32(10000)) td.sbx.UpdateValidator(val) }) }) diff --git a/tests/transaction_test.go b/tests/transaction_test.go index 827f1002a..583d6a7de 100644 --- a/tests/transaction_test.go +++ b/tests/transaction_test.go @@ -73,10 +73,6 @@ func TestTransactions(t *testing.T) { require.NoError(t, broadcastSendTransaction(t, tValKeys[tNodeIdx2][0], pubAlice.AccountAddress(), 80000000, 8000)) }) - t.Run("Invalid fee", func(t *testing.T) { - require.Error(t, broadcastSendTransaction(t, valKeyAlice, pubBob.AccountAddress(), 500000, 0)) - }) - t.Run("Alice tries double spending", func(t *testing.T) { require.NoError(t, broadcastSendTransaction(t, valKeyAlice, pubBob.AccountAddress(), 50000000, 5000)) diff --git a/txpool/config.go b/txpool/config.go index 6d07abf6f..f79d7775d 100644 --- a/txpool/config.go +++ b/txpool/config.go @@ -12,7 +12,7 @@ type Config struct { type FeeConfig struct { FixedFee float64 `toml:"fixed_fee"` - DailyLimit uint32 `toml:"daily_limit"` + DailyLimit int `toml:"daily_limit"` UnitPrice float64 `toml:"unit_price"` } @@ -40,16 +40,16 @@ func (conf *Config) BasicCheck() error { } } - if conf.Fee.DailyLimit == 0 { + if conf.Fee.DailyLimit <= 0 { return ConfigError{ - Reason: "dailyLimit can't be zero", + Reason: "dailyLimit should be positive", } } return nil } -func (conf *Config) CalculateConsumption() bool { +func (conf *Config) calculateConsumption() bool { return conf.Fee.UnitPrice > 0 } diff --git a/txpool/config_test.go b/txpool/config_test.go index 76ca0b5ce..035142d32 100644 --- a/txpool/config_test.go +++ b/txpool/config_test.go @@ -53,12 +53,21 @@ func TestConfigBasicCheck(t *testing.T) { { name: "Invalid DailyLimit", expectedErr: ConfigError{ - Reason: "dailyLimit can't be zero", + Reason: "dailyLimit should be positive", }, updateFn: func(c *Config) { c.Fee.DailyLimit = 0 }, }, + { + name: "Negative DailyLimit", + expectedErr: ConfigError{ + Reason: "dailyLimit should be positive", + }, + updateFn: func(c *Config) { + c.Fee.DailyLimit = -1 + }, + }, { name: "Valid Config", updateFn: func(c *Config) { diff --git a/txpool/errors.go b/txpool/errors.go index e8a29e480..6680bc28f 100644 --- a/txpool/errors.go +++ b/txpool/errors.go @@ -1,6 +1,10 @@ package txpool -import "fmt" +import ( + "fmt" + + "github.com/pactus-project/pactus/types/amount" +) // ConfigError is returned when the txPool configuration is invalid. type ConfigError struct { @@ -11,11 +15,11 @@ func (e ConfigError) Error() string { return e.Reason } -// AppendError is returned when the txPool configuration is invalid. -type AppendError struct { - Err error +// InvalidFeeError indicates that the transaction fee is below the minimum required. +type InvalidFeeError struct { + MinimumFee amount.Amount } -func (e AppendError) Error() string { - return fmt.Sprintf("unable to append transaction to pool: %s", e.Err) +func (e InvalidFeeError) Error() string { + return fmt.Sprintf("transaction fee is below the minimum of %s", e.MinimumFee) } diff --git a/txpool/txpool.go b/txpool/txpool.go index 0aa73d46e..35f11fe73 100644 --- a/txpool/txpool.go +++ b/txpool/txpool.go @@ -24,7 +24,7 @@ type txPool struct { config *Config sbx sandbox.Sandbox pools map[payload.Type]pool - consumptionMap map[crypto.Address]uint32 + consumptionMap map[crypto.Address]int broadcastCh chan message.Message store store.Reader logger *logger.SubLogger @@ -43,7 +43,7 @@ func NewTxPool(conf *Config, storeReader store.Reader, broadcastCh chan message. pool := &txPool{ config: conf, pools: pools, - consumptionMap: make(map[crypto.Address]uint32), + consumptionMap: make(map[crypto.Address]int), store: storeReader, broadcastCh: broadcastCh, } @@ -76,25 +76,40 @@ func (p *txPool) SetNewSandboxAndRecheck(sbx sandbox.Sandbox) { } } -// AppendTx validates the transaction and add it into the transaction pool -// without broadcast it. +// AppendTx validates the transaction and adds it to the transaction pool +// without broadcasting it. func (p *txPool) AppendTx(trx *tx.Tx) error { p.lk.Lock() defer p.lk.Unlock() - return p.appendTx(trx) + if err := p.checkTx(trx); err != nil { + return err + } + + if err := p.checkFee(trx); err != nil { + return err + } + + p.appendTx(trx) + + return nil } -// AppendTxAndBroadcast validates the transaction, add it into the transaction pool -// and broadcast it. +// AppendTxAndBroadcast validates the transaction, adds it to the transaction pool +// if the fee is acceptable, and broadcasts it regardless of the fee status. func (p *txPool) AppendTxAndBroadcast(trx *tx.Tx) error { p.lk.Lock() defer p.lk.Unlock() - if err := p.appendTx(trx); err != nil { + if err := p.checkTx(trx); err != nil { return err } + err := p.checkFee(trx) + if err == nil { + p.appendTx(trx) + } + go func(t *tx.Tx) { p.broadcastCh <- message.NewTransactionsMessage([]*tx.Tx{t}) }(trx) @@ -102,36 +117,27 @@ func (p *txPool) AppendTxAndBroadcast(trx *tx.Tx) error { return nil } -func (p *txPool) appendTx(trx *tx.Tx) error { +func (p *txPool) appendTx(trx *tx.Tx) { payloadType := trx.Payload().Type() payloadPool := p.pools[payloadType] - if payloadPool.list.Has(trx.ID()) { - p.logger.Trace("transaction is already in pool", "id", trx.ID()) - return nil - } + payloadPool.list.PushBack(trx.ID(), trx) + p.logger.Debug("transaction appended into pool", "trx", trx) +} +func (p *txPool) checkFee(trx *tx.Tx) error { if !trx.IsFreeTx() { minFee := p.estimatedMinimumFee(trx) if trx.Fee() < minFee { p.logger.Warn("low fee transaction", "txs", trx, "minFee", minFee) - return AppendError{ - Err: fmt.Errorf("low fee transaction, expected to be more than %s", minFee), + return InvalidFeeError{ + MinimumFee: minFee, } } } - if err := p.checkTx(trx); err != nil { - return AppendError{ - Err: err, - } - } - - payloadPool.list.PushBack(trx.ID(), trx) - p.logger.Debug("transaction appended into pool", "trx", trx) - return nil } @@ -162,30 +168,29 @@ func (p *txPool) HandleCommittedBlock(blk *block.Block) { p.removeTx(trx.ID()) } - if p.config.CalculateConsumption() { - for _, trx := range blk.Transactions() { - p.handleIncreaseConsumption(trx) - } - p.handleDecreaseConsumption(blk.Height()) + if p.config.calculateConsumption() { + p.increaseConsumption(blk) + p.decreaseConsumption(blk.Height()) } } -func (p *txPool) handleIncreaseConsumption(trx *tx.Tx) { - if trx.IsTransferTx() || trx.IsBondTx() || trx.IsWithdrawTx() { +func (p *txPool) increaseConsumption(blk *block.Block) { + // The first transaction is always the subsidy transaction. + for _, trx := range blk.Transactions()[1:] { signer := trx.Payload().Signer() - p.consumptionMap[signer] += uint32(trx.SerializeSize()) + p.consumptionMap[signer] += trx.SerializeSize() } } -func (p *txPool) handleDecreaseConsumption(height uint32) { +func (p *txPool) decreaseConsumption(curHeight uint32) { // If height is less than or equal to ConsumptionWindow, nothing to do. - if height <= p.config.ConsumptionWindow { + if curHeight <= p.config.ConsumptionWindow { return } // Calculate the block height that has passed out of the consumption window. - windowedBlockHeight := height - p.config.ConsumptionWindow + windowedBlockHeight := curHeight - p.config.ConsumptionWindow committedBlock, err := p.store.Block(windowedBlockHeight) if err != nil { p.logger.Error("failed to read block", "height", windowedBlockHeight, "err", err) @@ -200,20 +205,18 @@ func (p *txPool) handleDecreaseConsumption(height uint32) { return } - for _, trx := range blk.Transactions() { - if !trx.IsFreeTx() { - signer := trx.Payload().Signer() - if consumption, ok := p.consumptionMap[signer]; ok { - // Decrease the consumption by the size of the transaction - consumption -= uint32(trx.SerializeSize()) - - if consumption == 0 { - // If the new value is zero, remove the signer from the consumptionMap - delete(p.consumptionMap, signer) - } else { - // Otherwise, update the map with the new value - p.consumptionMap[signer] = consumption - } + for _, trx := range blk.Transactions()[1:] { + signer := trx.Payload().Signer() + if consumption, ok := p.consumptionMap[signer]; ok { + // Decrease the consumption by the size of the transaction + consumption -= trx.SerializeSize() + + if consumption <= 0 { + // If the new value is zero, remove the signer from the consumptionMap + delete(p.consumptionMap, signer) + } else { + // Otherwise, update the map with the new value + p.consumptionMap[signer] = consumption } } } @@ -299,6 +302,10 @@ func (p *txPool) Size() int { p.lk.RLock() defer p.lk.RUnlock() + return p.size() +} + +func (p *txPool) size() int { size := 0 for _, pool := range p.pools { size += pool.list.Size() @@ -317,9 +324,13 @@ func (p *txPool) fixedFee() amount.Amount { // consumptionalFee calculates based on the amount of data each address consumes daily. func (p *txPool) consumptionalFee(trx *tx.Tx) amount.Amount { - var consumption uint32 + if !p.config.calculateConsumption() { + return 0 + } + + var consumption int signer := trx.Payload().Signer() - txSize := uint32(trx.SerializeSize()) + txSize := trx.SerializeSize() if !p.store.HasPublicKey(signer) { consumption = p.config.Fee.DailyLimit @@ -338,7 +349,7 @@ func (p *txPool) AllPendingTxs() []*tx.Tx { p.lk.RLock() defer p.lk.RUnlock() - txs := make([]*tx.Tx, 0, p.Size()) + txs := make([]*tx.Tx, 0, p.size()) var next *linkedlist.Element[linkedmap.Pair[tx.ID, *tx.Tx]] for _, pool := range p.pools { @@ -353,18 +364,16 @@ func (p *txPool) AllPendingTxs() []*tx.Tx { return txs } -func (p *txPool) getPendingConsumption(signer crypto.Address) uint32 { - totalSize := uint32(0) +func (p *txPool) getPendingConsumption(signer crypto.Address) int { + totalSize := int(0) // TODO: big o is "o(n * m)" var next *linkedlist.Element[linkedmap.Pair[tx.ID, *tx.Tx]] - for ptype, pool := range p.pools { - if ptype == payload.TypeTransfer || ptype == payload.TypeBond || ptype == payload.TypeWithdraw { - for e := pool.list.HeadNode(); e != nil; e = next { - next = e.Next - if e.Data.Value.Payload().Signer() == signer { - totalSize += uint32(e.Data.Value.SerializeSize()) - } + for _, pool := range p.pools { + for e := pool.list.HeadNode(); e != nil; e = next { + next = e.Next + if e.Data.Value.Payload().Signer() == signer { + totalSize += e.Data.Value.SerializeSize() } } } diff --git a/txpool/txpool_test.go b/txpool/txpool_test.go index 0b1b4e854..5be70f9ed 100644 --- a/txpool/txpool_test.go +++ b/txpool/txpool_test.go @@ -6,14 +6,13 @@ import ( "time" "github.com/pactus-project/pactus/crypto" + "github.com/pactus-project/pactus/crypto/bls" "github.com/pactus-project/pactus/execution" "github.com/pactus-project/pactus/sandbox" - "github.com/pactus-project/pactus/store" "github.com/pactus-project/pactus/sync/bundle/message" - "github.com/pactus-project/pactus/types/account" "github.com/pactus-project/pactus/types/amount" "github.com/pactus-project/pactus/types/tx" - "github.com/pactus-project/pactus/types/validator" + "github.com/pactus-project/pactus/types/tx/payload" "github.com/pactus-project/pactus/util/logger" "github.com/pactus-project/pactus/util/testsuite" "github.com/stretchr/testify/assert" @@ -23,20 +22,23 @@ import ( type testData struct { *testsuite.TestSuite - pool *txPool - sbx *sandbox.MockSandbox - store *store.MockStore - ch chan message.Message + pool *txPool + sbx *sandbox.MockSandbox + ch chan message.Message } -func testConfig() *Config { +func testDefaultConfig() *Config { + return DefaultConfig() +} + +func testConsumptionalConfig() *Config { return &Config{ MaxSize: 10, ConsumptionWindow: 3, Fee: &FeeConfig{ - FixedFee: 0.01, - DailyLimit: 280, - UnitPrice: 0.00005, + FixedFee: 0, + DailyLimit: 360, + UnitPrice: 0.000005, }, } } @@ -48,21 +50,21 @@ func setup(t *testing.T, cfg *Config) *testData { broadcastCh := make(chan message.Message, 10) sbx := sandbox.MockingSandbox(ts) - config := testConfig() + config := testDefaultConfig() if cfg != nil { config = cfg } - mockStore := store.MockingStore(ts) - poolInt := NewTxPool(config, mockStore, broadcastCh) + poolInt := NewTxPool(config, sbx.TestStore, broadcastCh) poolInt.SetNewSandboxAndRecheck(sbx) pool := poolInt.(*txPool) assert.NotNil(t, pool) + sbx.TestAcceptSortition = true + return &testData{ TestSuite: ts, pool: pool, sbx: sbx, - store: mockStore, ch: broadcastCh, } } @@ -92,119 +94,203 @@ func (td *testData) shouldPublishTransaction(t *testing.T, txID tx.ID) { } } +// makeValidTransferTx makes a valid Transfer transaction for testing purpose. +func (td *testData) makeValidTransferTx(options ...func(tm *testsuite.TransactionMaker)) *tx.Tx { + options = append(options, testsuite.TransactionWithLockTime(td.sbx.CurrentHeight())) + trx := td.GenerateTestTransferTx(options...) + signer := trx.Payload().Signer() + + acc := td.sbx.MakeNewAccount(signer) + acc.AddToBalance(trx.Payload().Value() + trx.Fee()) + td.sbx.UpdateAccount(signer, acc) + + return trx +} + +// makeValidBondTx makes a valid Bond transaction for testing purpose. +func (td *testData) makeValidBondTx(options ...func(tm *testsuite.TransactionMaker)) *tx.Tx { + options = append(options, testsuite.TransactionWithLockTime(td.sbx.CurrentHeight())) + trx := td.GenerateTestBondTx(options...) + signer := trx.Payload().Signer() + + acc := td.sbx.MakeNewAccount(signer) + acc.AddToBalance(trx.Payload().Value() + trx.Fee()) + td.sbx.UpdateAccount(signer, acc) + + return trx +} + +// makeValidUnbondTx makes a valid Unbond transaction for testing purpose. +// Ensure that the signer key is set through the options. +func (td *testData) makeValidUnbondTx(options ...func(tm *testsuite.TransactionMaker)) *tx.Tx { + options = append(options, testsuite.TransactionWithLockTime(td.sbx.CurrentHeight())) + trx := td.GenerateTestUnbondTx(options...) + + validatorPublicKey := trx.PublicKey().(*bls.PublicKey) + val := td.sbx.MakeNewValidator(validatorPublicKey) + td.sbx.UpdateValidator(val) + + return trx +} + +// makeValidUnbondTx makes a valid Withdraw transaction for testing purpose. +// Ensure that the signer key is set through the options. +func (td *testData) makeValidWithdrawTx(options ...func(tm *testsuite.TransactionMaker)) *tx.Tx { + options = append(options, testsuite.TransactionWithLockTime(td.sbx.CurrentHeight())) + trx := td.GenerateTestWithdrawTx(options...) + + validatorPublicKey := trx.PublicKey().(*bls.PublicKey) + val := td.sbx.MakeNewValidator(validatorPublicKey) + val.AddToStake(trx.Payload().Value() + trx.Fee()) + val.UpdateUnbondingHeight(td.sbx.CurrentHeight() - (td.sbx.TestParams.UnbondInterval + 1)) + td.sbx.UpdateValidator(val) + + return trx +} + +// makeValidSortitionTx makes a valid Sortition transaction for testing purpose. +// Ensure that the signer key is set through the options. +func (td *testData) makeValidSortitionTx(options ...func(tm *testsuite.TransactionMaker)) *tx.Tx { + options = append(options, testsuite.TransactionWithLockTime(td.sbx.CurrentHeight())) + trx := td.GenerateTestSortitionTx(options...) + + validatorPublicKey := trx.PublicKey().(*bls.PublicKey) + val := td.sbx.MakeNewValidator(validatorPublicKey) + val.UpdateLastBondingHeight(td.sbx.CurrentHeight() - (td.sbx.TestParams.BondInterval + 1)) + td.sbx.UpdateValidator(val) + + return trx +} + func TestAppendAndRemove(t *testing.T) { td := setup(t, nil) - height := td.RandHeight() - td.sbx.TestStore.AddTestBlock(height) - testTrx := tx.NewSubsidyTx(height+1, td.RandAccAddress(), 1e9) + trx := td.makeValidTransferTx() + + assert.NoError(t, td.pool.AppendTx(trx)) + assert.True(t, td.pool.HasTx(trx.ID())) + assert.Equal(t, trx, td.pool.PendingTx(trx.ID())) - assert.NoError(t, td.pool.AppendTx(testTrx)) - assert.True(t, td.pool.HasTx(testTrx.ID())) - assert.Equal(t, testTrx, td.pool.PendingTx(testTrx.ID())) + td.pool.removeTx(trx.ID()) + assert.False(t, td.pool.HasTx(trx.ID()), "Transaction should be removed") + assert.Nil(t, td.pool.PendingTx(trx.ID())) +} + +func TestAppendSameTransaction(t *testing.T) { + td := setup(t, nil) - // Appending the same transaction again, should not return any error - assert.NoError(t, td.pool.AppendTx(testTrx)) + trx := td.makeValidTransferTx() - td.pool.removeTx(testTrx.ID()) - assert.False(t, td.pool.HasTx(testTrx.ID()), "Transaction should be removed") - assert.Nil(t, td.pool.PendingTx(testTrx.ID())) + err := td.pool.AppendTx(trx) + assert.NoError(t, err) + + err = td.pool.AppendTx(trx) + assert.ErrorIs(t, err, execution.TransactionCommittedError{ID: trx.ID()}) +} + +func TestDisableConsumption(t *testing.T) { + td := setup(t, testDefaultConfig()) + + trx := td.GenerateTestTransferTx() + assert.Zero(t, td.pool.consumptionalFee(trx)) } func TestCalculatingConsumption(t *testing.T) { - td := setup(t, nil) + td := setup(t, testConsumptionalConfig()) // Generate keys for different transaction signers _, prv1 := td.RandEd25519KeyPair() - _, prv2 := td.RandEd25519KeyPair() - _, prv3 := td.RandBLSKeyPair() - _, prv4 := td.RandBLSKeyPair() + pub2, prv2 := td.RandEd25519KeyPair() + pub3, prv3 := td.RandBLSKeyPair() + pub4, prv4 := td.RandBLSKeyPair() // Generate different types of transactions - trx11 := td.GenerateTestTransferTx(testsuite.TransactionWithEd25519Signer(prv1)) - trx12 := td.GenerateTestBondTx(testsuite.TransactionWithEd25519Signer(prv1)) - trx13 := td.GenerateTestWithdrawTx(testsuite.TransactionWithBLSSigner(prv3)) - trx14 := td.GenerateTestUnbondTx(testsuite.TransactionWithBLSSigner(prv4)) - trx21 := td.GenerateTestTransferTx(testsuite.TransactionWithEd25519Signer(prv2)) - trx31 := td.GenerateTestBondTx(testsuite.TransactionWithBLSSigner(prv4)) - trx41 := td.GenerateTestWithdrawTx(testsuite.TransactionWithBLSSigner(prv3)) - trx42 := td.GenerateTestTransferTx(testsuite.TransactionWithEd25519Signer(prv2)) + trx10 := tx.NewSubsidyTx(1, td.RandAccAddress(), 1e9) + trx11 := td.makeValidTransferTx(testsuite.TransactionWithEd25519Signer(prv1)) + trx12 := td.makeValidBondTx(testsuite.TransactionWithEd25519Signer(prv2)) + trx13 := td.makeValidUnbondTx(testsuite.TransactionWithBLSSigner(prv4)) + trx20 := tx.NewSubsidyTx(2, td.RandAccAddress(), 1e9) + trx21 := td.makeValidTransferTx(testsuite.TransactionWithEd25519Signer(prv1)) + trx30 := tx.NewSubsidyTx(3, td.RandAccAddress(), 1e9) + trx31 := td.makeValidBondTx(testsuite.TransactionWithBLSSigner(prv3)) + trx32 := td.makeValidSortitionTx(testsuite.TransactionWithBLSSigner(prv4)) + trx40 := tx.NewSubsidyTx(4, td.RandAccAddress(), 1e9) + trx41 := td.makeValidUnbondTx(testsuite.TransactionWithBLSSigner(prv3)) + trx42 := td.makeValidTransferTx(testsuite.TransactionWithEd25519Signer(prv2)) + trx50 := tx.NewSubsidyTx(5, td.RandAccAddress(), 1e9) + trx51 := td.makeValidWithdrawTx(testsuite.TransactionWithBLSSigner(prv3)) + trx52 := td.makeValidTransferTx(testsuite.TransactionWithEd25519Signer(prv2)) + + // Commit the first block + blk1, cert1 := td.GenerateTestBlock(1, + testsuite.BlockWithTransactions([]*tx.Tx{trx10, trx11, trx12, trx13})) + td.sbx.TestStore.SaveBlock(blk1, cert1) // Expected consumption map after transactions - expected := map[crypto.Address]uint32{ - prv2.PublicKeyNative().AccountAddress(): uint32(trx21.SerializeSize()) + uint32(trx42.SerializeSize()), - prv4.PublicKeyNative().AccountAddress(): uint32(trx31.SerializeSize()), - prv3.PublicKeyNative().ValidatorAddress(): uint32(trx41.SerializeSize()), + diff2 := 0 + if trx42.SerializeSize() > trx12.SerializeSize() { + diff2 = trx42.SerializeSize() - trx12.SerializeSize() + } + + expected := map[crypto.Address]int{ + pub2.AccountAddress(): trx52.SerializeSize() + diff2, + pub3.AccountAddress(): trx31.SerializeSize(), + pub3.ValidatorAddress(): trx41.SerializeSize() + trx51.SerializeSize(), + pub4.ValidatorAddress(): trx32.SerializeSize() - trx13.SerializeSize(), } tests := []struct { height uint32 txs []*tx.Tx }{ - {1, []*tx.Tx{trx11, trx12, trx13, trx14}}, - {2, []*tx.Tx{trx21}}, - {3, []*tx.Tx{trx31}}, - {4, []*tx.Tx{trx41, trx42}}, + {2, []*tx.Tx{trx20, trx21}}, + {3, []*tx.Tx{trx30, trx31, trx32}}, + {4, []*tx.Tx{trx40, trx41, trx42}}, + {5, []*tx.Tx{trx50, trx51, trx52}}, } for _, tt := range tests { // Generate a block with the transactions for the given height - blk, cert := td.GenerateTestBlock(tt.height, func(bm *testsuite.BlockMaker) { - bm.Txs = tt.txs - }) - td.store.SaveBlock(blk, cert) + blk, cert := td.GenerateTestBlock(tt.height, testsuite.BlockWithTransactions(tt.txs)) + td.sbx.TestStore.SaveBlock(blk, cert) // Handle the block in the transaction pool td.pool.HandleCommittedBlock(blk) + + t.Logf("consumption Map: %v\n", td.pool.consumptionMap) } require.Equal(t, expected, td.pool.consumptionMap) } func TestEstimatedConsumptionalFee(t *testing.T) { - td := setup(t, &Config{ - MaxSize: 10, - ConsumptionWindow: 3, - Fee: &FeeConfig{ - FixedFee: 0, - DailyLimit: 360, - UnitPrice: 0.000005, - }, - }) + td := setup(t, testConsumptionalConfig()) t.Run("Test indexed signer", func(t *testing.T) { - accPub, accPrv := td.RandEd25519KeyPair() - acc1Addr := accPub.AccountAddress() - acc1 := account.NewAccount(0) - acc1.AddToBalance(1000e9) - td.sbx.UpdateAccount(acc1Addr, acc1) - - txr := td.GenerateTestTransferTx(testsuite.TransactionWithEd25519Signer(accPrv), testsuite.TransactionWithAmount(1e9)) - - blk, cert := td.GenerateTestBlock(td.RandHeight(), testsuite.BlockWithTransactions([]*tx.Tx{txr})) - td.store.SaveBlock(blk, cert) + _, accPrv := td.RandEd25519KeyPair() + trx := td.makeValidTransferTx(testsuite.TransactionWithEd25519Signer(accPrv)) + blk, cert := td.GenerateTestBlock(td.RandHeight(), testsuite.BlockWithTransactions([]*tx.Tx{trx})) + td.sbx.TestStore.SaveBlock(blk, cert) tests := []struct { - value amount.Amount fee amount.Amount withErr bool }{ - {1e9, 0, false}, - {1e9, 0, false}, - {1e9, 89800000, false}, - {1e9, 90000000, false}, - {1e9, 7000000000, false}, - {1e9, 0, true}, + {0, false}, + {0, false}, + {2_310_000, false}, + {3_090_000, false}, + {7_740_000, false}, + {9_300_000, false}, + {0, true}, } for _, tt := range tests { - trx := td.GenerateTestTransferTx( + testTrx := td.makeValidTransferTx( testsuite.TransactionWithEd25519Signer(accPrv), - testsuite.TransactionWithAmount(tt.value), - testsuite.TransactionWithFee(tt.fee), - ) + testsuite.TransactionWithFee(tt.fee)) - err := td.pool.AppendTx(trx) + err := td.pool.AppendTx(testTrx) if tt.withErr { assert.Error(t, err) } else { @@ -214,13 +300,7 @@ func TestEstimatedConsumptionalFee(t *testing.T) { }) t.Run("Test non-indexed signer", func(t *testing.T) { - _, accPrv := td.RandEd25519KeyPair() - - trx := td.GenerateTestTransferTx( - testsuite.TransactionWithEd25519Signer(accPrv), - testsuite.TransactionWithAmount(1e9), - testsuite.TransactionWithFee(0), - ) + trx := td.makeValidTransferTx(testsuite.TransactionWithFee(0)) err := td.pool.AppendTx(trx) assert.Error(t, err) @@ -231,27 +311,24 @@ func TestAppendInvalidTransaction(t *testing.T) { td := setup(t, nil) invTrx := td.GenerateTestTransferTx() - assert.Error(t, td.pool.AppendTx(invTrx)) + + err := td.pool.AppendTx(invTrx) + assert.Error(t, err) } // TestFullPool tests if the pool prunes the old transactions when it is full. func TestFullPool(t *testing.T) { - td := setup(t, nil) + conf := testDefaultConfig() + conf.MaxSize = 10 + td := setup(t, conf) - randHeight := td.RandHeight() - _ = td.sbx.TestStore.AddTestBlock(randHeight) trxs := make([]*tx.Tx, td.pool.config.transferPoolSize()+1) - senderAddr := td.RandAccAddress() - senderAcc := account.NewAccount(0) - senderAcc.AddToBalance(1000e9) - td.sbx.UpdateAccount(senderAddr, senderAcc) - // Make sure the pool is empty assert.Equal(t, 0, td.pool.Size()) for i := 0; i < len(trxs); i++ { - trx := tx.NewTransferTx(randHeight+1, senderAddr, td.RandAccAddress(), 1e9, 1000_000_000) + trx := td.makeValidTransferTx() assert.NoError(t, td.pool.AppendTx(trx)) trxs[i] = trx @@ -265,48 +342,23 @@ func TestFullPool(t *testing.T) { func TestEmptyPool(t *testing.T) { td := setup(t, nil) - assert.Empty(t, td.pool.PrepareBlockTransactions(), "pool should be empty") + trxs := td.pool.AllPendingTxs() + assert.Empty(t, trxs, "pool should be empty") } func TestPrepareBlockTransactions(t *testing.T) { td := setup(t, nil) - randHeight := td.RandHeight() + td.sbx.TestParams.UnbondInterval - _ = td.sbx.TestStore.AddTestBlock(randHeight) - - acc1Addr := td.RandAccAddress() - acc1 := account.NewAccount(0) - acc1.AddToBalance(1000e9) - td.sbx.UpdateAccount(acc1Addr, acc1) - - val1PubKey, _ := td.RandBLSKeyPair() - val1 := validator.NewValidator(val1PubKey, 0) - val1.AddToStake(1000e9) - td.sbx.UpdateValidator(val1) - - val2PubKey, _ := td.RandBLSKeyPair() - val2 := validator.NewValidator(val2PubKey, 0) - val2.AddToStake(1000e9) - val2.UpdateUnbondingHeight(1) - td.sbx.UpdateValidator(val2) - - val3PubKey, _ := td.RandBLSKeyPair() - val3 := validator.NewValidator(val3PubKey, 0) - val3.AddToStake(1000e9) - td.sbx.UpdateValidator(val3) - - transferTx := tx.NewTransferTx(randHeight+1, acc1Addr, td.RandAccAddress(), 1e9, 100_000_000) - - pub, _ := td.RandBLSKeyPair() - bondTx := tx.NewBondTx(randHeight+2, acc1Addr, pub.ValidatorAddress(), pub, 1e9, 100_000_000) - - unbondTx := tx.NewUnbondTx(randHeight+3, val1.Address()) - - withdrawTx := tx.NewWithdrawTx(randHeight+4, val2.Address(), td.RandAccAddress(), 1e9, 100_000_000) + pub1, _ := td.RandBLSKeyPair() + _, prv2 := td.RandBLSKeyPair() + _, prv3 := td.RandBLSKeyPair() + _, prv4 := td.RandBLSKeyPair() - td.sbx.TestAcceptSortition = true - sortitionTx := tx.NewSortitionTx(randHeight, val3.Address(), - td.RandProof()) + transferTx := td.makeValidTransferTx() + bondTx := td.makeValidBondTx(testsuite.TransactionWithValidatorPublicKey(pub1)) + unbondTx := td.makeValidUnbondTx(testsuite.TransactionWithBLSSigner(prv2)) + withdrawTx := td.makeValidWithdrawTx(testsuite.TransactionWithBLSSigner(prv3)) + sortitionTx := td.makeValidSortitionTx(testsuite.TransactionWithBLSSigner(prv4)) assert.NoError(t, td.pool.AppendTx(transferTx)) assert.NoError(t, td.pool.AppendTx(unbondTx)) @@ -323,46 +375,104 @@ func TestPrepareBlockTransactions(t *testing.T) { assert.Equal(t, transferTx.ID(), trxs[4].ID()) } -func TestAppendAndBroadcast(t *testing.T) { - td := setup(t, nil) +func TestAddSubsidyTransactions(t *testing.T) { + t.Run("invalid transaction: Should return error", func(t *testing.T) { + td := setup(t, nil) + + randHeight := td.RandHeight() + td.sbx.TestStore.AddTestBlock(randHeight) + trx := tx.NewSubsidyTx(randHeight, td.RandAccAddress(), 1e9) + + err := td.pool.AppendTx(trx) + assert.ErrorIs(t, err, execution.LockTimeExpiredError{ + LockTime: randHeight, + }) + }) - height := td.RandHeight() - td.sbx.TestStore.AddTestBlock(height) - testTrx := tx.NewSubsidyTx(height+1, td.RandAccAddress(), 1e9) + t.Run("valid transaction: Should add it to the pool", func(t *testing.T) { + td := setup(t, nil) - assert.NoError(t, td.pool.AppendTxAndBroadcast(testTrx)) - td.shouldPublishTransaction(t, testTrx.ID()) + trx := tx.NewSubsidyTx(td.RandHeight(), td.RandAccAddress(), 1e9) - invTrx := td.GenerateTestBondTx() - assert.Error(t, td.pool.AppendTxAndBroadcast(invTrx)) + err := td.pool.AppendTx(trx) + assert.NoError(t, err) + }) } -func TestAddSubsidyTransactions(t *testing.T) { +func TestRecheckTransactions(t *testing.T) { td := setup(t, nil) - randHeight := td.RandHeight() - td.sbx.TestStore.AddTestBlock(randHeight) - proposer1 := td.RandAccAddress() - proposer2 := td.RandAccAddress() - trx1 := tx.NewSubsidyTx(randHeight, proposer1, 1e9, tx.WithMemo("subsidy-tx-1")) - trx2 := tx.NewSubsidyTx(randHeight+1, proposer1, 1e9, tx.WithMemo("subsidy-tx-1")) - trx3 := tx.NewSubsidyTx(randHeight+1, proposer2, 1e9, tx.WithMemo("subsidy-tx-2")) - - err := td.pool.AppendTx(trx1) - assert.ErrorIs(t, err, AppendError{ - Err: execution.LockTimeExpiredError{ - LockTime: randHeight, - }, - }) + trx := tx.NewSubsidyTx(td.RandHeight(), td.RandAccAddress(), 1e9) - err = td.pool.AppendTx(trx2) + err := td.pool.AppendTx(trx) assert.NoError(t, err) + assert.Equal(t, 1, td.pool.Size()) - err = td.pool.AppendTx(trx3) - assert.NoError(t, err) + td.pool.SetNewSandboxAndRecheck(td.sbx) + assert.Equal(t, 0, td.pool.Size()) +} - td.sbx.TestStore.AddTestBlock(randHeight + 1) +func TestAppendAndBroadcast(t *testing.T) { + t.Run("Invalid transaction: Should return error", func(t *testing.T) { + td := setup(t, nil) - td.pool.SetNewSandboxAndRecheck(td.sbx) - assert.Zero(t, td.pool.Size()) + invTrx := td.GenerateTestTransferTx() + assert.Error(t, td.pool.AppendTxAndBroadcast(invTrx)) + }) + + t.Run("Valid transaction with valid fee: Should add to pool and broadcast", func(t *testing.T) { + td := setup(t, nil) + + trx := td.makeValidTransferTx() + + err := td.pool.AppendTxAndBroadcast(trx) + assert.NoError(t, err) + + assert.Equal(t, 1, td.pool.Size()) + td.shouldPublishTransaction(t, trx.ID()) + }) + + t.Run("Valid transaction with zero fee: Should broadcast but not add to the pool", func(t *testing.T) { + td := setup(t, nil) + + trx := td.makeValidTransferTx(testsuite.TransactionWithFee(0)) + + err := td.pool.AppendTxAndBroadcast(trx) + assert.NoError(t, err) + + assert.Zero(t, td.pool.Size()) + td.shouldPublishTransaction(t, trx.ID()) + }) +} + +func TestAllPendingTxs(t *testing.T) { + td := setup(t, nil) + + trxs := td.pool.AllPendingTxs() + assert.Empty(t, trxs, 0) + + pub1, _ := td.RandBLSKeyPair() + _, prv2 := td.RandBLSKeyPair() + + transferTx := td.makeValidTransferTx() + bondTx := td.makeValidBondTx(testsuite.TransactionWithValidatorPublicKey(pub1)) + unbondTx := td.makeValidUnbondTx() + withdrawTx := td.makeValidWithdrawTx() + sortitionTx := td.makeValidSortitionTx(testsuite.TransactionWithBLSSigner(prv2)) + + assert.NoError(t, td.pool.AppendTx(transferTx)) + assert.NoError(t, td.pool.AppendTx(bondTx)) + assert.NoError(t, td.pool.AppendTx(unbondTx)) + assert.NoError(t, td.pool.AppendTx(withdrawTx)) + assert.NoError(t, td.pool.AppendTx(sortitionTx)) + + trxs = td.pool.AllPendingTxs() + assert.Len(t, trxs, 5) +} + +func TestEstimatedFee(t *testing.T) { + td := setup(t, nil) + + estimatedFee := td.pool.EstimatedFee(td.RandAmount(), payload.TypeTransfer) + assert.Equal(t, td.pool.config.fixedFee(), estimatedFee) } diff --git a/types/block/block_test.go b/types/block/block_test.go index ada6cb875..d566ccce2 100644 --- a/types/block/block_test.go +++ b/types/block/block_test.go @@ -200,7 +200,7 @@ func TestBasicCheck(t *testing.T) { "02" + // Tx[0]: Flags "01" + // Tx[0]: Version "01000000" + // Tx[0]: LockTime - "01" + // Tx[0]: Fee + "00" + // Tx[0]: Fee "00" + // Tx[0]: Memo "01" + // Tx[0]: PayloadType "00" + // Tx[0]: Sender (treasury) diff --git a/types/tx/tx.go b/types/tx/tx.go index ae02d45b3..d56cef460 100644 --- a/types/tx/tx.go +++ b/types/tx/tx.go @@ -148,19 +148,14 @@ func (tx *Tx) BasicCheck() error { Reason: "lock time is not defined", } } - if tx.Payload().Value() < 0 || tx.Payload().Value() > amount.MaxNanoPAC { - return BasicCheckError{ - Reason: fmt.Sprintf("invalid amount: %s", tx.Payload().Value()), - } - } - if tx.Fee() < 0 || tx.Fee() > amount.MaxNanoPAC { + if len(tx.Memo()) > maxMemoLength { return BasicCheckError{ - Reason: fmt.Sprintf("invalid fee: %s", tx.Fee()), + Reason: fmt.Sprintf("memo length exceeded: %d", len(tx.Memo())), } } - if len(tx.Memo()) > maxMemoLength { + if tx.Payload().Value() < 0 || tx.Payload().Value() > amount.MaxNanoPAC { return BasicCheckError{ - Reason: fmt.Sprintf("memo length exceeded: %d", len(tx.Memo())), + Reason: fmt.Sprintf("invalid amount: %s", tx.Payload().Value()), } } if err := tx.Payload().BasicCheck(); err != nil { @@ -168,6 +163,9 @@ func (tx *Tx) BasicCheck() error { Reason: fmt.Sprintf("invalid payload: %s", err.Error()), } } + if err := tx.checkFee(); err != nil { + return err + } if err := tx.checkSignature(); err != nil { return err } @@ -177,6 +175,23 @@ func (tx *Tx) BasicCheck() error { return nil } +func (tx *Tx) checkFee() error { + if tx.Fee() < 0 || tx.Fee() > amount.MaxNanoPAC { + return BasicCheckError{ + Reason: fmt.Sprintf("invalid fee: %s", tx.Fee()), + } + } + if tx.IsFreeTx() { + if tx.Fee() != 0 { + return BasicCheckError{ + Reason: fmt.Sprintf("invalid fee: %s", tx.Fee()), + } + } + } + + return nil +} + func (tx *Tx) checkSignature() error { if tx.IsSubsidyTx() { // Ensure no signatory is set for subsidy transactions. diff --git a/types/tx/tx_test.go b/types/tx/tx_test.go index 0a9ec15f1..c0304c3b5 100644 --- a/types/tx/tx_test.go +++ b/types/tx/tx_test.go @@ -143,24 +143,6 @@ func TestBasicCheck(t *testing.T) { }) }) - t.Run("Invalid fee", func(t *testing.T) { - trx := tx.NewTransferTx(ts.RandHeight(), ts.RandAccAddress(), ts.RandAccAddress(), 1, -1) - - err := trx.BasicCheck() - assert.ErrorIs(t, err, tx.BasicCheckError{ - Reason: "invalid fee: -0.000000001 PAC", - }) - }) - - t.Run("Invalid fee", func(t *testing.T) { - trx := tx.NewTransferTx(ts.RandHeight(), ts.RandAccAddress(), ts.RandAccAddress(), 1, (42e15)+1) - - err := trx.BasicCheck() - assert.ErrorIs(t, err, tx.BasicCheckError{ - Reason: "invalid fee: 42000000 PAC", - }) - }) - t.Run("Invalid signer address", func(t *testing.T) { valKey := ts.RandValKey() trx := tx.NewTransferTx(ts.RandHeight(), ts.RandAccAddress(), ts.RandAccAddress(), 1, 1) @@ -487,3 +469,61 @@ func TestIsFreeTx(t *testing.T) { assert.False(t, trx4.IsFreeTx()) assert.True(t, trx5.IsFreeTx()) } + +func TestCheckFee(t *testing.T) { + ts := testsuite.NewTestSuite(t) + + tests := []struct { + name string + trx *tx.Tx + expectedErr error + }{ + { + name: "Negative fee", + trx: ts.GenerateTestTransferTx( + testsuite.TransactionWithFee(-1)), + expectedErr: tx.BasicCheckError{Reason: "invalid fee: -0.000000001 PAC"}, + }, + { + name: "Big fee", + trx: ts.GenerateTestTransferTx( + testsuite.TransactionWithFee(42e15 + 1)), + expectedErr: tx.BasicCheckError{Reason: "invalid fee: 42000000 PAC"}, + }, + { + name: "Subsidy transaction with fee", + trx: tx.NewTransferTx(ts.RandHeight(), crypto.TreasuryAddress, ts.RandAccAddress(), ts.RandAmount(), 1), + expectedErr: tx.BasicCheckError{Reason: "invalid fee: 0.000000001 PAC"}, + }, + { + name: "Subsidy transaction with zero fee", + trx: tx.NewTransferTx(ts.RandHeight(), crypto.TreasuryAddress, ts.RandAccAddress(), ts.RandAmount(), 0), + expectedErr: nil, + }, + { + name: "Transfer transaction with zero fee", + trx: ts.GenerateTestTransferTx( + testsuite.TransactionWithFee(0)), + expectedErr: nil, + }, + { + name: "Transfer transaction with non-zero fee", + trx: ts.GenerateTestTransferTx( + testsuite.TransactionWithFee(1)), + expectedErr: nil, + }, + { + name: "Unbond transaction with zero fee", + trx: ts.GenerateTestUnbondTx( + testsuite.TransactionWithFee(0)), + expectedErr: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.trx.BasicCheck() + assert.ErrorIs(t, err, tt.expectedErr) + }) + } +} diff --git a/util/testsuite/testsuite.go b/util/testsuite/testsuite.go index e3d8fa412..1877df390 100644 --- a/util/testsuite/testsuite.go +++ b/util/testsuite/testsuite.go @@ -39,8 +39,8 @@ func GenerateSeed() int64 { return time.Now().UTC().UnixNano() } -// NewTestSuiteForSeed creates a new TestSuite with the given seed. -func NewTestSuiteForSeed(seed int64) *TestSuite { +// NewTestSuiteFromSeed creates a new TestSuite with the given seed. +func NewTestSuiteFromSeed(seed int64) *TestSuite { return &TestSuite{ Seed: seed, //nolint:gosec // to reproduce the failed tests @@ -157,7 +157,7 @@ func (ts *TestSuite) RandRound() int16 { return ts.RandInt16(10) } -// RandAmount returns a random amount between [0, max). +// RandAmount returns a random amount between [1e9, max). // If max is not set, it defaults to 1000e9. func (ts *TestSuite) RandAmount(max ...amount.Amount) amount.Amount { maxAmt := amount.Amount(1000e9) // default max amount @@ -165,7 +165,7 @@ func (ts *TestSuite) RandAmount(max ...amount.Amount) amount.Amount { maxAmt = max[0] } - return ts.RandAmountRange(0, maxAmt) + return ts.RandAmountRange(1e9, maxAmt) } // RandAmountRange returns a random amount between [min, max). @@ -175,7 +175,7 @@ func (ts *TestSuite) RandAmountRange(min, max amount.Amount) amount.Amount { return amt + min } -// RandFee returns a random fee between [0, max). +// RandFee returns a random fee between [1e7, max). // If max is not set, it defaults to 1e9. func (ts *TestSuite) RandFee(max ...amount.Amount) amount.Amount { maxFee := amount.Amount(1e9) // default max fee @@ -183,7 +183,7 @@ func (ts *TestSuite) RandFee(max ...amount.Amount) amount.Amount { maxFee = max[0] } - return ts.RandAmountRange(0, maxFee) + return ts.RandAmountRange(1e7, maxFee) } // RandBytes returns a slice of random bytes of the given length. @@ -792,8 +792,4 @@ func (*TestSuite) HelperSignTransaction(prv crypto.PrivateKey, trx *tx.Tx) { sig := prv.Sign(trx.SignBytes()) trx.SetSignature(sig) trx.SetPublicKey(prv.PublicKey()) - - if err := trx.BasicCheck(); err != nil { - panic(err) - } } diff --git a/wallet/tx_builder.go b/wallet/tx_builder.go index 975304d2b..d2fccb2de 100644 --- a/wallet/tx_builder.go +++ b/wallet/tx_builder.go @@ -10,8 +10,10 @@ import ( "github.com/pactus-project/pactus/types/tx/payload" ) +// TxOption defines a function type used to apply options to a txBuilder. type TxOption func(builder *txBuilder) error +// OptionLockTime sets the lock time for the transaction. func OptionLockTime(lockTime uint32) func(builder *txBuilder) error { return func(builder *txBuilder) error { builder.lockTime = lockTime @@ -20,14 +22,33 @@ func OptionLockTime(lockTime uint32) func(builder *txBuilder) error { } } +// OptionFeeFromString sets the transaction fee using a string input. +func OptionFeeFromString(feeStr string) func(builder *txBuilder) error { + return func(builder *txBuilder) error { + if feeStr == "" { + return nil + } + + fee, err := amount.FromString(feeStr) + if err != nil { + return err + } + builder.fee = &fee + + return nil + } +} + +// OptionFee sets the transaction fee using an Amount input. func OptionFee(fee amount.Amount) func(builder *txBuilder) error { return func(builder *txBuilder) error { - builder.fee = fee + builder.fee = &fee return nil } } +// OptionMemo sets a memo or note for the transaction. func OptionMemo(memo string) func(builder *txBuilder) error { return func(builder *txBuilder) error { builder.memo = memo @@ -36,18 +57,20 @@ func OptionMemo(memo string) func(builder *txBuilder) error { } } +// txBuilder helps build and configure a transaction before submitting it. type txBuilder struct { client *grpcClient - from *crypto.Address - to *crypto.Address + sender *crypto.Address + receiver *crypto.Address pub *bls.PublicKey typ payload.Type lockTime uint32 amount amount.Amount - fee amount.Amount + fee *amount.Amount memo string } +// newTxBuilder initializes a txBuilder with provided options, allowing for flexible configuration of the transaction. func newTxBuilder(client *grpcClient, options ...TxOption) (*txBuilder, error) { builder := &txBuilder{ client: client, @@ -62,26 +85,29 @@ func newTxBuilder(client *grpcClient, options ...TxOption) (*txBuilder, error) { return builder, nil } -func (m *txBuilder) setFromAddr(addr string) error { - from, err := crypto.AddressFromString(addr) +// setSenderAddr sets the sender's address for the transaction. +func (m *txBuilder) setSenderAddr(addr string) error { + sender, err := crypto.AddressFromString(addr) if err != nil { return err } - m.from = &from + m.sender = &sender return nil } -func (m *txBuilder) setToAddress(addr string) error { - to, err := crypto.AddressFromString(addr) +// setReceiverAddress sets the recipient's address for the transaction. +func (m *txBuilder) setReceiverAddress(addr string) error { + receiver, err := crypto.AddressFromString(addr) if err != nil { return err } - m.to = &to + m.receiver = &receiver return nil } +// build constructs and finalizes the transaction, selecting the appropriate type based on the builder's configuration. func (m *txBuilder) build() (*tx.Tx, error) { err := m.setLockTime() if err != nil { @@ -96,21 +122,21 @@ func (m *txBuilder) build() (*tx.Tx, error) { var trx *tx.Tx switch m.typ { case payload.TypeTransfer: - trx = tx.NewTransferTx(m.lockTime, *m.from, *m.to, m.amount, m.fee, tx.WithMemo(m.memo)) + trx = tx.NewTransferTx(m.lockTime, *m.sender, *m.receiver, m.amount, *m.fee, tx.WithMemo(m.memo)) case payload.TypeBond: pub := m.pub - val, _ := m.client.getValidator(m.to.String()) + val, _ := m.client.getValidator(m.receiver.String()) if val != nil { // validator exists pub = nil } - trx = tx.NewBondTx(m.lockTime, *m.from, *m.to, pub, m.amount, m.fee, tx.WithMemo(m.memo)) + trx = tx.NewBondTx(m.lockTime, *m.sender, *m.receiver, pub, m.amount, *m.fee, tx.WithMemo(m.memo)) case payload.TypeUnbond: - trx = tx.NewUnbondTx(m.lockTime, *m.from, tx.WithMemo(m.memo)) + trx = tx.NewUnbondTx(m.lockTime, *m.sender, tx.WithMemo(m.memo)) case payload.TypeWithdraw: - trx = tx.NewWithdrawTx(m.lockTime, *m.from, *m.to, m.amount, m.fee, tx.WithMemo(m.memo)) + trx = tx.NewWithdrawTx(m.lockTime, *m.sender, *m.receiver, m.amount, *m.fee, tx.WithMemo(m.memo)) case payload.TypeSortition: return nil, fmt.Errorf("unable to build sortition transactions") @@ -119,6 +145,8 @@ func (m *txBuilder) build() (*tx.Tx, error) { return trx, nil } +// setLockTime assigns a lock time to the transaction. +// If not provided, it retrieves the last block height and increments it. func (m *txBuilder) setLockTime() error { if m.lockTime == 0 { if m.client == nil { @@ -135,8 +163,10 @@ func (m *txBuilder) setLockTime() error { return nil } +// setFee determines the fee for the transaction. +// If not set, it retrieves the fee from the client based on amount and transaction type. func (m *txBuilder) setFee() error { - if m.fee == 0 { + if m.fee == nil { if m.client == nil { return ErrOffline } @@ -144,7 +174,7 @@ func (m *txBuilder) setFee() error { if err != nil { return err } - m.fee = fee + m.fee = &fee } return nil diff --git a/wallet/wallet.go b/wallet/wallet.go index 83a863bea..f9eb54e57 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -255,11 +255,11 @@ func (w *Wallet) MakeTransferTx(sender, receiver string, amt amount.Amount, if err != nil { return nil, err } - err = maker.setFromAddr(sender) + err = maker.setSenderAddr(sender) if err != nil { return nil, err } - err = maker.setToAddress(receiver) + err = maker.setReceiverAddress(receiver) if err != nil { return nil, err } @@ -277,11 +277,11 @@ func (w *Wallet) MakeBondTx(sender, receiver, pubKey string, amt amount.Amount, if err != nil { return nil, err } - err = maker.setFromAddr(sender) + err = maker.setSenderAddr(sender) if err != nil { return nil, err } - err = maker.setToAddress(receiver) + err = maker.setReceiverAddress(receiver) if err != nil { return nil, err } @@ -310,7 +310,7 @@ func (w *Wallet) MakeUnbondTx(addr string, opts ...TxOption) (*tx.Tx, error) { if err != nil { return nil, err } - err = maker.setFromAddr(addr) + err = maker.setSenderAddr(addr) if err != nil { return nil, err } @@ -328,11 +328,11 @@ func (w *Wallet) MakeWithdrawTx(sender, receiver string, amt amount.Amount, if err != nil { return nil, err } - err = maker.setFromAddr(sender) + err = maker.setSenderAddr(sender) if err != nil { return nil, err } - err = maker.setToAddress(receiver) + err = maker.setReceiverAddress(receiver) if err != nil { return nil, err } diff --git a/www/grpc/server_test.go b/www/grpc/server_test.go index 1970a65d8..721e8238b 100644 --- a/www/grpc/server_test.go +++ b/www/grpc/server_test.go @@ -50,7 +50,7 @@ func setup(t *testing.T, conf *Config) *testData { conf = testConfig() } - ts := testsuite.NewTestSuiteForSeed(1717506021599855072) + ts := testsuite.NewTestSuite(t) // for saving test wallets in temp directory err := os.Chdir(util.TempDirPath())