Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Feb 1, 2021

Conversation

trinhdn97
Copy link
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 thang14 and lewtran 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
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

types/bloom9.go Outdated Show resolved Hide resolved
@lewtran
Copy link
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