Skip to content

Commit

Permalink
consensus: Remove signatures from V2TransactionWeight
Browse files Browse the repository at this point in the history
Signatures are expensive to verify, so counting them towards the
transaction's weight seemed like a reasonable thing to do. In
practice, though, this is quite tricky, with questionable benefits.

If "weight" is represented as a single integer, then we need to
choose coefficients for the storage cost and signature cost; what
should those coefficients be? There's no obvious way to choose them,
especially considering that the compute cost is one-time, whereas
the storage cost is indefinite.

Alternatively, if we make "weight" a struct with fields for each
resource, then the txpool and miner code becomes significantly more
complicated. Which should a miner prefer: a large transaction with
few signatures, or a small transaction with many signatures? Again,
there are various options, but it's not obvious which is best.

As always, it's worth seeing what Bitcoin does here. Bitcoin has
a MAX_BLOCK_SIGOPS limit, currently set at 20,000. Since Bitcoin
blocks also have a size limit of 1MB, they can have at most 1
signature per 50 bytes. In Sia's case, signatures are 64 bytes,
and our size limit is 2MB, so blocks can have at most 1 signature
per 32 bytes. And yet, we have no explicit MAX_BLOCK_SIGOPS! The
reason Bitcoin needs an explicit limit is Bitcoin Script, which
allows you to cause a signature verification call without having
to put an actual signature in the transaction. This is not the
case in Sia, so the problem is sidestepped.

Anyway, what's the attack here? Someone can make a big transaction
with 100,000 signatures, and (if they pay enough fees) thereby cause
every node to run at max CPU for... a few seconds. There's no real
threat of disruption unless blocks start taking longer to verify than
they do to mine, which would require close to 1 GB of signatures.
This is probably why we've never observed such an attack on Sia:
there just isn't much point.
  • Loading branch information
lukechampine committed Oct 4, 2023
1 parent 65d3947 commit 00ded87
Showing 1 changed file with 1 addition and 21 deletions.
22 changes: 1 addition & 21 deletions consensus/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,27 +288,7 @@ func (s State) V2TransactionWeight(txn types.V2Transaction) uint64 {
a.EncodeTo(e)
}
e.WriteBytes(txn.ArbitraryData)
storage := uint64(wc.n)

var signatures int
for _, sci := range txn.SiacoinInputs {
signatures += len(sci.SatisfiedPolicy.Signatures)
}
for _, sfi := range txn.SiafundInputs {
signatures += len(sfi.SatisfiedPolicy.Signatures)
}
signatures += 2 * len(txn.FileContracts)
signatures += 2 * len(txn.FileContractRevisions)
for _, fcr := range txn.FileContractResolutions {
switch fcr.Resolution.(type) {
case *types.V2FileContractRenewal, *types.V2FileContractFinalization:
signatures += 2
}
}
signatures += len(txn.Attestations)

// TODO: choose coefficients empirically
return storage + 100*uint64(signatures)
return uint64(wc.n)
}

// FileContractTax computes the tax levied on a given contract.
Expand Down

0 comments on commit 00ded87

Please sign in to comment.