Skip to content

Implement logs bloom filters / minor fix RPC API, tx pool#130

Merged
lewtran merged 5 commits into
masterfrom
fix/block_tx_APIs
Feb 1, 2021
Merged

Implement logs bloom filters / minor fix RPC API, tx pool#130
lewtran merged 5 commits into
masterfrom
fix/block_tx_APIs

Conversation

@trinhdn97
Copy link
Copy Markdown
Contributor

@trinhdn97 trinhdn97 commented Jan 27, 2021

  • Fix block API and remove status field in public transaction
  • Move and public keccakState interface from trie package to crypto for bloom types to use
  • Fix off-by-one error when blending the new-errors into the error-slice in the txpool.
  • Free pointer from slice after popping element from price heap. As per the documentation for golang heap, heap Pop can cause a memory leak if the element that is returned is a pointer and is not set to nil. This causes the pointer to be held by the underlying array, such that it cannot be garbage collected.
  • Optimize bloom filters, fix incorrect Add(), Test(), and TestBytes(), make these function take []byte instead of a big.Int.

Bloom.Add() takes a big.Integer, then calls d.Bytes() to get a byte representation of the integer. It then hashes that byte representation, and uses the hash to populate the bloom filter. The problem is that if your value is an address, integer, or some other value that doesn't occupy the full 32 bytes, it gets truncated to the number of bytes that it has, and a hash of a truncated value is different than a hash of the same value with leading 0's.

So, for example, if I tried to add the value 0x000000000000000000000000000000000000000000000001a055690d9db80000 to my bloom filter using bloom.Add(value), the value that got hashed would actually be 0x01a055690d9db80000.

The bloom.Test() function does the same thing, as does bloom.TestBytes(). So if I have a Bloom filter that properly entered the topic 0x000000000000000000000000000000000000000000000001a055690d9db80000 as a 32 byte topic, and I run bloom.TestBytes(value) (where value is the bytes corresponding to the above topic), it will truncate the value before hashing and end up returning false.

It seems that neither of these functions are actually used by Geth. Bloom filters are created with CreateBloom(), which doesn't use .Add(), and tested with BloomLookup() which doesn't use Test or TestBytes. Both CreateBloom() and BloomLookup() treat 32 byte topics properly, but the corresponding BloomFilter functions do not.

  • Fix null pointer exception in estimate gas API

@trinhdn97 trinhdn97 requested review from lewtran and thang14 January 27, 2021 16:25
@trinhdn97 trinhdn97 changed the title Fix get block APIs and remove status field in public transaction Minor fix API, tx pool and bloom filters Jan 29, 2021
Copy link
Copy Markdown
Contributor

@lewtran lewtran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add in-depth use cases of Bloom Filter in PR description

Comment thread types/bloom9.go Outdated
@lewtran
Copy link
Copy Markdown
Contributor

lewtran commented Feb 1, 2021

LGTM

@lewtran lewtran merged commit 62b1746 into master Feb 1, 2021
@trinhdn97 trinhdn97 deleted the fix/block_tx_APIs branch March 1, 2021 11:01
@lewtran lewtran changed the title Minor fix API, tx pool and bloom filters Implement logs bloom filters / minor fix RPC API, tx pool Mar 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants