zcash/lightwalletd

Implement ZIP 244 txid logic for v5 transactions

str4d opened this issue · 3 comments

str4d commented

Support for parsing v5 transactions (as specified in ZIP 225) was added in #367. However, the txid computation for v5 transactions (as specified in ZIP 244) was not implemented. This means that at present, lightwalletd returns incorrect txids for v5 transactions in compact blocks:

block := parser.NewBlock()
rest, err := block.ParseFromSlice(blockData)
if err != nil {
return nil, errors.Wrap(err, "error parsing block")
}
if len(rest) != 0 {
return nil, errors.New("received overlong message")
}
if block.GetHeight() != height {
return nil, errors.New("received unexpected height block")
}
return block.ToCompact(), nil

// ToCompact returns the compact representation of the full block.
func (b *Block) ToCompact() *walletrpc.CompactBlock {
compactBlock := &walletrpc.CompactBlock{
//TODO ProtoVersion: 1,
Height: uint64(b.GetHeight()),
PrevHash: b.hdr.HashPrevBlock,
Hash: b.GetEncodableHash(),
Time: b.hdr.Time,
}
// Only Sapling transactions have a meaningful compact encoding
saplingTxns := make([]*walletrpc.CompactTx, 0, len(b.vtx))
for idx, tx := range b.vtx {
if tx.HasShieldedElements() {
saplingTxns = append(saplingTxns, tx.ToCompact(idx))
}
}
compactBlock.Vtx = saplingTxns
return compactBlock
}

// ToCompact converts the given (full) transaction to compact format.
func (tx *Transaction) ToCompact(index int) *walletrpc.CompactTx {
ctx := &walletrpc.CompactTx{
Index: uint64(index), // index is contextual
Hash: tx.GetEncodableHash(),

// GetDisplayHash returns the transaction hash in big-endian display order.
func (tx *Transaction) GetDisplayHash() []byte {
if tx.cachedTxID != nil {
return tx.cachedTxID
}
// SHA256d
digest := sha256.Sum256(tx.rawBytes)
digest = sha256.Sum256(digest[:])
// Convert to big-endian
tx.cachedTxID = Reverse(digest[:])
return tx.cachedTxID
}
// GetEncodableHash returns the transaction hash in little-endian wire format order.
func (tx *Transaction) GetEncodableHash() []byte {
digest := sha256.Sum256(tx.rawBytes)
digest = sha256.Sum256(digest[:])
return digest[:]
}

str4d commented

#367 had test vectors that include txids; these passed because the test conditional was backwards, and checking that the derived txid didn't match the test vector.

if bytes.Equal(tx.cachedTxID, []byte(txtestdata.Txid)) {
t.Fatal("txid")
}

str4d commented

Reopening because this shouldn't have been closed by #393 (which only partially fixed the problem, the internal code still derives the wrong txid and needs fixing).

@str4d, prompted by your recent comment, I began looking into this and discovered that the Golang version of blake2b doesn't have the personalization interface. Investigating this, I found that you had already opened an issue against this 4 years ago: golang/go#32447 (comment)

Since this has still not been implemented, I went ahead and wrote a patch to zcashd's getblock RPC to add another verbosity level, 3, that returns exactly the information that lightwalletd needs. I know you don't like this, but it bothers me greatly that lightwalletd makes two RPC calls for each block it fetches (one at verbosity=1 to get the txids, and another at verbosity=0 to get the raw block hex). The lightwalletd sync now takes about 12 hours on my high-end desktop, and reducing the number of RPCs by half is sure to help.

Think of it as a short-term improvement until Golang can implement blake2 properly.

The lightwalletd PR is #449 and the corresponding zcashd PR is zcash/zcash#6747. (These can be merged and deployed in either order.)