>
> An option would be that the height is included in the scriptSig for all
>> transactions, but for non-coinbase transctions, the height used is zero.
>>
> No need to add an extra field to the transaction just to include the
> height. We can just add a rule that the height specified in the scriptSig
> in coinbase transactions (and only coinbase transactions) is copied into
> the locktime of the transaction before computing the normalized transaction
> ID and leave the locktime untouched for all normal transactions
>

No need to replace lock times (or any other part of the transaction) at
all. If you have to, just serialize the height right before serializing the
transaction (into the same buffer). And you could pre-serialize 0 instead
of the height for all non-coinbase transactions. I don't really see what
that gets you, though, because the 0 is not really doing anything.

But, I don't see any reason you have to mess with the serialization this
much at all. Just do:

uint256 normalized_txid(CTransaction tx)
{
  // Coinbase transactions are already normalized
  if (!tx.IsCoinbase())
  {
    foreach(CTxIn in : tx.vin)
    {
      if (!ReplacePrevoutHashWithNormalizedHash(in.prevout))
        throw NormalizationError("Could not lookup prevout");
      in.scriptSig.clear();
    }
  }

  // Serialize
  CHashWriter ss(SER_GETHASH, 0);
  ss << tx;
  return ss.GetHash();
}

An alternative could be (although I like the above option better):

uint256 normalized_txid(CTransaction tx, int nHeight)
{
  foreach(CTxIn in : tx.vin)
  {
    if (!in.prevout.IsNull() &&
!ReplacePrevoutHashWithNormalizedHash(in.prevout))
      throw NormalizationError("Could not lookup prevout");
    in.scriptSig.clear();
  }

  // Serialize
  CHashWriter ss(SER_GETHASH, 0);

if (tx.IsCoinbase())
ss << nHeight;
// or:
// ss << (tx.IsCoinbase() ? nHeight : 0);

  ss << tx;
  return ss.GetHash();
}