Skip to content

Commit 8da8834

Browse files
committed
Add server-side check for insanely high fees.
Goal is to prevent a situation like decred/dcrwallet#2000 from happening again even if users are running the buggy client code. While here, the fee calculation is fixed to consider the cost of input scripts to redeem P2PKH outputs. Even if this is not the case, minimum fee requirements in dcrd use the same assumption.
1 parent ee00e49 commit 8da8834

File tree

2 files changed

+35
-10
lines changed

2 files changed

+35
-10
lines changed

coinjoin/coinjoin.go

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,23 @@ func (t *Tx) UnmarshalBinary(b []byte) error {
205205
return t.Tx.Deserialize(bytes.NewReader(b))
206206
}
207207

208+
var bogusSigScript = make([]byte, 108) // worst case size to redeem a P2PKH
209+
210+
func (t *Tx) estimateSerializeSize(tx *wire.MsgTx, mcount int) int {
211+
bogusMixedOut := &wire.TxOut{
212+
Value: t.mixValue,
213+
Version: t.sc.version(),
214+
PkScript: make([]byte, t.sc.scriptSize()),
215+
}
216+
for _, in := range tx.TxIn {
217+
in.SignatureScript = bogusSigScript
218+
}
219+
for i := 0; i < mcount; i++ {
220+
tx.AddTxOut(bogusMixedOut)
221+
}
222+
return tx.SerializeSize()
223+
}
224+
208225
func feeForSerializeSize(relayFeePerKb int64, txSerializeSize int) int64 {
209226
fee := relayFeePerKb * int64(txSerializeSize) / 1000
210227

@@ -220,6 +237,17 @@ func feeForSerializeSize(relayFeePerKb int64, txSerializeSize int) int64 {
220237
return fee
221238
}
222239

240+
// highFeeRate is the maximum multiplier of the standard fee rate before unmixed
241+
// data is refused for paying too high of a fee. It should not be too low such
242+
// that mixing outputs at the smallest common mixed value errors for too high
243+
// fees, as these mixes are performed without any change outputs.
244+
const highFeeRate = 150
245+
246+
func paysHighFees(fee, relayFeePerKb int64, txSerializeSize int) bool {
247+
maxFee := feeForSerializeSize(highFeeRate*relayFeePerKb, txSerializeSize)
248+
return fee > maxFee
249+
}
250+
223251
func (t *Tx) ValidateUnmixed(unmixed []byte, mcount int) error {
224252
var fee int64
225253
other := new(wire.MsgTx)
@@ -253,18 +281,15 @@ func (t *Tx) ValidateUnmixed(unmixed []byte, mcount int) error {
253281
return err
254282
}
255283
fee -= int64(mcount) * t.mixValue
256-
bogusMixedOut := &wire.TxOut{
257-
Value: t.mixValue,
258-
Version: t.sc.version(),
259-
PkScript: make([]byte, t.sc.scriptSize()),
260-
}
261-
for i := 0; i < mcount; i++ {
262-
other.AddTxOut(bogusMixedOut)
263-
}
264-
requiredFee := feeForSerializeSize(t.feeRate, other.SerializeSize())
284+
size := t.estimateSerializeSize(other, mcount)
285+
requiredFee := feeForSerializeSize(t.feeRate, size)
265286
if fee < requiredFee {
266287
return errors.New("coinjoin: unmixed transaction does not pay enough network fees")
267288
}
289+
if paysHighFees(fee, t.feeRate, size) {
290+
return errors.New("coinjoin: unmixed transaction pays insanely high fees")
291+
}
292+
268293
return nil
269294
}
270295

integration/honest_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ func TestHonest(t *testing.T) {
196196
i := i
197197
go func() {
198198
input := &wire.TxIn{ValueIn: inputValue * 1e8}
199-
change := &wire.TxOut{Value: 1e8 - int64(1+i)*0.001e8, PkScript: change}
199+
change := &wire.TxOut{Value: 1e8 - 0.0001e8 + int64(i+1), PkScript: change}
200200
con := newConfirmer(input, change)
201201
conn, err := tls.Dial("tcp", s.Addr, nettest.ClientTLS)
202202
if err != nil {

0 commit comments

Comments
 (0)