Switch AllowHighFee to MaxFeeRate #1415
Conversation
|
Seems we also need to update the |
|
Thanks for reviewing. I updated as requested. |
vctt94
force-pushed
the
switchMaxFeeRate
branch
from
ce6d7c0
to
97f3660
Compare
rpcserver.go
Outdated
| @@ -3299,6 +3299,14 @@ func handleSearchRawTransactions(s *rpcServer, cmd interface{}, closeChan <-chan | |||
| // handleSendRawTransaction implements the sendrawtransaction command. | |||
| func handleSendRawTransaction(s *rpcServer, cmd interface{}, closeChan <-chan struct{}) (interface{}, error) { | |||
| c := cmd.(*btcjson.SendRawTransactionCmd) | |||
| if s.cfg.TxIndex == nil { | |||
There was a problem hiding this comment.
Seems like a pretty big downgrade if we now require the txindex just to send a transaction over the interface. Instead, during validation in the mempool, we know the direct fee since it's stored as part of the mempool transaction entry. So we can put the logic deeper into the mempool, accepting an optional argument of the max fee that the resulting transaction should be allowed to have.
rpcserver.go
Outdated
| @@ -3319,6 +3327,23 @@ func handleSendRawTransaction(s *rpcServer, cmd interface{}, closeChan <-chan st | |||
|
|
|||
| // Use 0 for the tag to represent local node. | |||
| tx := btcutil.NewTx(&msgTx) | |||
|
|
|||
| var totalSatoshiIn int64 | |||
| inputs, err := fetchInputTxos(s, &msgTx) | |||
There was a problem hiding this comment.
Alternatively, we can just check the UTXO set here. As if the inputs are already spent, it'll be rejected within the mempool anyway.
rpcserver.go
Outdated
| totalSatoshiOut += txOut.Value | ||
| } | ||
| txFee := totalSatoshiIn - totalSatoshiOut | ||
| if txFee > int64(*c.MaxFeeRate) { |
There was a problem hiding this comment.
this is comparing two different things. the limit is a feerate in sats/kB, which is being compared to the total fee in sats
rpcserver.go
Outdated
| txFee := totalSatoshiIn - totalSatoshiOut | ||
| if txFee > int64(*c.MaxFeeRate) { | ||
| return nil, &btcjson.RPCError{ | ||
| Code: btcjson.ErrRPCDeserialization, |
There was a problem hiding this comment.
doesn't seem like the correct response code, since the request was successfully deserialized
There was a problem hiding this comment.
returning wire.RejectInvalid now. I believe it is more appropriate
rpcserverhelp.go
Outdated
| "sendrawtransaction--result0": "The hash of the transaction", | ||
| "sendrawtransaction--synopsis": "Submits the serialized, hex-encoded transaction to the local peer and relays it to the network.", | ||
| "sendrawtransaction-hextx": "Serialized, hex-encoded signed transaction", | ||
| "sendrawtransaction-maxfeerate": "Reject transactions whose fee rate is higher than the specified value. The value is given in satoshis", |
There was a problem hiding this comment.
if the intent is to match the bitcoind implementation, the maxfeerate is in sats/kB and not satoshis
https://github.com/bitcoin/bitcoin/pull/13541/files#diff-01aa7d1d32f1b9e5a836c9c411978918R1043
vctt94
force-pushed
the
switchMaxFeeRate
branch
2 times, most recently
from
828f10d
to
ec2398a
Compare
vctt94
force-pushed
the
switchMaxFeeRate
branch
4 times, most recently
from
7e53c0f
to
ec1adc1
Compare
vctt94
force-pushed
the
switchMaxFeeRate
branch
from
ec1adc1
to
0ba7230
Compare
|
Thanks for pointing that Roasbeef. I moved the maxFeeRate to the Also fixed the others comments |
|
@jcvernaleo (as per #1530)
|
|
Not sure why GitHub closed this upon merge of another PR. Re-opening, sorry. |
|
Okay, this seems to be a bug with the GitHub UI, will try again later. |
|
Okay, I'm giving up. No idea why GitHub doesn't let me re-open the PR, sorry. |
After commit bitcoin/bitcoin@c033c4b bitcoin switched allowHighFee to maxFeeRate on sendrawtransaction rpc command.
This PR switches AllowHighFees to MaxFeeRate as well.