service-my-wallet-v3 icon indicating copy to clipboard operation
service-my-wallet-v3 copied to clipboard

Unnecessary Default 10000 fee for MakePayment function

Open Can-Sahin opened this issue 6 years ago • 6 comments

Using Service v0.26.0

Why is there a default fixed 10000 fee addition in makePayment function in case options are empty

File api.js

MerchantAPI.prototype.makePayment = function (guid, options) {
    if (!isNaN(options.fee_per_byte)) {
        if (options.fee_per_byte < 50) warning = warnings.LOW_FEE_PER_BYTE
        payment.updateFeePerKb(options.fee_per_byte)
      } else if (!isNaN(options.fee)) {
        warning = warnings.STATIC_FEE_AMOUNT
        payment.fee(options.fee)
      } else {
        warning = warnings.USING_DEFAULT_FEE
        payment.fee(10000) // WHATS THE REASON OF THIS FALLBACK?
      }

Because when fee is unset, blockchain-wallet-client automatically sets fee per byte dynamically from its api API for a regular speed which is much more preferable.

Whats the point of this ? This forces developer to calculate fee_per_byte beforehand where not calculating could easily lead to a regular transaction. If there is no harm deleting it I would do a PR.

Can-Sahin avatar May 11 '18 17:05 Can-Sahin

This fallback was for compatibility with our legacy wallet API, which would use 10000 satoshi when no fee was specified.

I agree that it would be useful to use the fee per byte values from our fee API. Did you have any thoughts on how that might look? One possibility is to allow setting the fee_per_byte parameter to 'regular' or 'priority' and use that info to determine the correct fee.

jtormey avatar May 11 '18 17:05 jtormey

Yes I was thinking what you have said. I can suggest something like (dirty)

Before creating payment get fees from the API. Then add another IF condition to check fee_per_byte (you need to change the type of fee_per_byte option at server.js. Its not only number anymore)

MerchantAPI.prototype.makePayment = function (guid, options) {
  return this.getWallet(guid, options)
    .then(requireSecondPassword(options))
    // Get fees from API
    .then(function (wallet) {
      return Promise.resolve(
        Blockchain.API.getFees().then(fees => {
          wallet.dynamicFees = fees
          return wallet
        }).catch(err => wallet)
      )
    })
    .then(function (wallet) {
      var from = isNaN(options.from)
        ? options.from : parseInt(options.from)

      var payment = wallet.createPayment()
        .to(options.to)
        .amount(options.amount)
        .from(from)

      var warning
      var dynamicFee = wallet.dynamicFees && wallet.dynamicFees[options.fee_per_byte]
      if (wallet.dynamicFees && isNaN(options.fee_per_byte) && !isNaN(dynamicFee)) {
          payment.updateFeePerKb(fee)
      } else if (!isNaN(options.fee_per_byte)) {
        if (options.fee_per_byte < 50) warning = warnings.LOW_FEE_PER_BYTE
        payment.updateFeePerKb(options.fee_per_byte)
      } else if (!isNaN(options.fee)) {
        warning = warnings.STATIC_FEE_AMOUNT
        payment.fee(options.fee)
      } else {
        warning = warnings.USING_DEFAULT_FEE
        payment.fee(10000)
      }

But I have another question. Is there a way to calculate transaction fee (in BTC) using 'service-my-wallet' ? I know fee depends on the transaction size and size depends on the inputs/outputs. So it isn't straight forward. Now I can only get it by creating a payment without publishing it (Modified the code myself). Payment objects contains finalFee property which I need to show to my customer. Would you considering adding another interface to do this? Just like the makePayment function but without publishing it, so that payment properties are accessible beforehand

Can-Sahin avatar May 11 '18 19:05 Can-Sahin

Thanks for the example, will work on adding that this week!

It is inconvenient not to know the final fee until the transaction is published. Maybe there could be an option to perform a dry run, which makes a payment but does not publish. The dry run call could include a token that you pass to a second request when you want to continue with the payment.

jtormey avatar May 16 '18 19:05 jtormey

Ok. It would be nice. I am already doing it now. I created my self a special condition if the payment 'to' is empty then I just build the payment and return it. For dynamic fees or for this I can do a pull request if that works as well let me know.

Can-Sahin avatar May 16 '18 21:05 Can-Sahin

Has this issue been fixed?

rockyxcoded avatar Jul 03 '19 02:07 rockyxcoded

เมื่อ พ. 3 ก.ค. 2019 เวลา 09:16 Fulfil Nelson [email protected] เขียนว่า:

Has this issue been fixed?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/blockchain/service-my-wallet-v3/issues/351?email_source=notifications&email_token=AG4YJ5HK47M5DYCSG3OQ5E3P5QDX5A5CNFSM4E7PEROKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZDBV7I#issuecomment-507910909, or mute the thread https://github.com/notifications/unsubscribe-auth/AG4YJ5FVYHZUUIOGM5GLOL3P5QDX5ANCNFSM4E7PEROA .

prasitchai1991 avatar Jul 03 '19 02:07 prasitchai1991