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

Fix memory leak cache set item #23

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

antazoey
Copy link
Contributor

Summary

Fixes a bug preventing the cache from saving an ens address lookup correctly (or at all).
Also fixes a few spelling errors I found when debugging this problem.

Other Information

Addresses an issue I raised earlier today: #22
See the issue for more details!

@cerealkill
Copy link
Contributor

cerealkill commented Sep 15, 2020

Hi @unparalleled-js 😄 , nice catch, and thanks for the MR!

I have checked the submitted code and it might seem to solve the issue but it creates other problems because it limits the size of the "key" to 8 bytes and the C library expects to version the contract address and chain id in the key. See examples in my computer:

$ ll ~/.in3
total 144
drwxr-xr-x  14 paul   448B Sep 15 08:50 .
drwxr-xr-x+ 55 paul   1.7K Sep 15 08:59 ..
-rw-r--r--   1 paul   5.2K Jul 17 06:40 C00000000000c2e074ec69a0dfb2997ba6c7d2e1e
-rw-r--r--   1 paul   9.1K Sep 15 08:57 C226159d
-rw-r--r--   1 paul    14K Sep 15 08:57 C4976fb0
-rw-r--r--   1 paul    20B Sep 15 08:57 ens:depr
-rw-r--r--   1 paul    20B Jul 17 06:40 ens:depraz.eth:2
-rw-r--r--   1 paul    20B Aug 19 15:52 ens:depraz.eth:2:1
-rw-r--r--   1 paul    20B Sep 15 08:57 ens:vita
-rw-r--r--   1 paul   2.3K Sep 15 08:57 nodelist
-rw-r--r--   1 paul   231B Aug 26 16:17 nodelist_1_0xac1b824795e1eb1f6e609fe0da9b9af8beaab60f
-rw-r--r--   1 paul   542B Aug 26 15:35 nodelist_246_0x039562872008f7a76674a6e7842804f0ad37cb13
-rw-r--r--   1 paul   552B Aug 17 15:30 nodelist_42_0x4c396dcf50ac396e5fdea18163251699b5fcca25
-rw-r--r--   1 paul   822B Aug 31 15:32 nodelist_5_0x5f51e413581dd76759e9eed51e63d14c8d1379c8

What used to be nodelist_1_0xac1b824795e1eb1f6e609fe0da9b9af8beaab60f - a mask of nodelist_chainId_ethAddress - became just nodelist. Also the contract code hash was shortened into C226159d and so on.

We need to investigate deeper to find why the C lib is creating non-utf8 characters or Python is finding more characters than it should in the string. It would also be great to write tests that catch this issue.

@antazoey
Copy link
Contributor Author

Hi @unparalleled-js 😄 , nice catch, and thanks for the MR!

I have checked the submitted code and it might seem to solve the issue but it creates other problems because it limits the size of the "key" to 8 bytes and the C library expects to version the contract address and chain id in the key. See examples in my computer:

$ ll ~/.in3
total 144
drwxr-xr-x  14 paul   448B Sep 15 08:50 .
drwxr-xr-x+ 55 paul   1.7K Sep 15 08:59 ..
-rw-r--r--   1 paul   5.2K Jul 17 06:40 C00000000000c2e074ec69a0dfb2997ba6c7d2e1e
-rw-r--r--   1 paul   9.1K Sep 15 08:57 C226159d
-rw-r--r--   1 paul    14K Sep 15 08:57 C4976fb0
-rw-r--r--   1 paul    20B Sep 15 08:57 ens:depr
-rw-r--r--   1 paul    20B Jul 17 06:40 ens:depraz.eth:2
-rw-r--r--   1 paul    20B Aug 19 15:52 ens:depraz.eth:2:1
-rw-r--r--   1 paul    20B Sep 15 08:57 ens:vita
-rw-r--r--   1 paul   2.3K Sep 15 08:57 nodelist
-rw-r--r--   1 paul   231B Aug 26 16:17 nodelist_1_0xac1b824795e1eb1f6e609fe0da9b9af8beaab60f
-rw-r--r--   1 paul   542B Aug 26 15:35 nodelist_246_0x039562872008f7a76674a6e7842804f0ad37cb13
-rw-r--r--   1 paul   552B Aug 17 15:30 nodelist_42_0x4c396dcf50ac396e5fdea18163251699b5fcca25
-rw-r--r--   1 paul   822B Aug 31 15:32 nodelist_5_0x5f51e413581dd76759e9eed51e63d14c8d1379c8

What used to be nodelist_1_0xac1b824795e1eb1f6e609fe0da9b9af8beaab60f - a mask of nodelist_chainId_ethAddress - became just nodelist. Also the contract code hash was shortened into C226159d and so on.

We need to investigate deeper to find why the C lib is creating non-utf8 characters or Python is finding more characters than it should in the string. It would also be great to write tests that catch this issue.

Wondering if there is any update on this issue or somewhere I can track it. If not, going to look into it more, as would be nice to have it resolved on my end.

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.

None yet

2 participants