NebulousLabs / Sia

Blockchain-based marketplace for file storage. Project has moved to GitLab: https://gitlab.com/NebulousLabs/Sia
https://sia.tech
MIT License
2.71k stars 440 forks source link

Feature: Subtract transaction fees from sent amount in POST /wallet/siacoins API #2216

Open Fornax96 opened 7 years ago

Fornax96 commented 7 years ago

Right now when a Siacoin transaction is sent the fees are calculated and paid from the remaining balance in the wallet. But this isn't possible when trying to send all the coins in the wallet. It's also very hard to predict the fees you need to subtract from the amount beforehand. Before 1.3 I used to just subtract 10 SC from the amount and it would always work, but now it still leaves some coins behind when I do that.

Most other cryptocurrency wallets have a button to empty the wallet, which creates a transaction with all coins in the wallet to an address.

That's why I would like a boolean parameter for the POST /wallet/siacoins API that allows the fees to be subtracted from the transaction amount. That way I can empty a wallet and make sure there's not a single Hasting left behind.

lukechampine commented 7 years ago

For contributors interested in tackling this, the implementation should be pretty straightforward. The fee is calculated here. Instead of calling txnBuilder.FundSiacoins(amount.Add(tpoolFee)), you can just call txnBuilder.FundSiacoins(amount). (Of course, you'll need to check that amount > tpoolFee first.)

hudaniel commented 6 years ago

Should this boolean also be used for multi-sending?

ach1000 commented 6 years ago

Subtract fees from sent amount.

$ git diff
diff --git a/modules/wallet/money.go b/modules/wallet/money.go
index e2259c82..7a86688a 100644
--- a/modules/wallet/money.go
+++ b/modules/wallet/money.go
@@ -101,7 +101,7 @@ func (w *Wallet) SendSiacoins(amount types.Currency, dest types.UnlockHash) (txn
        _, tpoolFee := w.tpool.FeeEstimation()
        tpoolFee = tpoolFee.Mul64(750) // Estimated transaction size in bytes
        output := types.SiacoinOutput{
-               Value:      amount,
+               Value:      amount.Sub(tpoolFee),
                UnlockHash: dest,
        }

@@ -111,7 +111,7 @@ func (w *Wallet) SendSiacoins(amount types.Currency, dest types.UnlockHash) (txn
                        txnBuilder.Drop()
                }
        }()
-       err = txnBuilder.FundSiacoins(amount.Add(tpoolFee))
+       err = txnBuilder.FundSiacoins(amount)
        if err != nil {
                w.log.Println("Attempt to send coins has failed - failed to fund transaction:", err)
                return nil, build.ExtendErr("unable to fund transaction", err)
lukechampine commented 6 years ago

@ach1000 this doesn't address the other half of the issue. It should still be possible to have the wallet choose the fee for you, since that's a lot more convenient. And we need to support both varieties in the API.

I actually made a PR for this a while ago, but it's outdated now. If someone cleans it up and resubmits it, I'm sure we can get it merged: https://github.com/NebulousLabs/Sia/pull/1849/files

nielscastien commented 6 years ago

@lukechampine I am looking for a little project that I can help with and this one looks like it's doable for someone like me. Is this something that is still requested by the community? If so, I would like to start with re-submitting your PR to implement manual set fees. After that we can discuss how to implement the emptying of the wallet in one go, as requested by @Fornax96 . Maybe add an endpoint like /wallet/empty?address=xxxxx&type=siacoins. What do you think?