First crack at serialization with backrefs. #119
Conversation
| assert len(b) == 19124 | ||
|
|
||
| def test_deserialize_bomb(self): | ||
| def make_bomb(depth): |
There was a problem hiding this comment.
I think a more challenging bomb is where each leaf is a unique (but large) subset of the atom. But I suppose the python implementation might not deduplicate substr.
There was a problem hiding this comment.
Deduplicating substr is a much bigger challenge, since the number of substrings in a string of length n is zlib does it; maybe it just builds a reverse table of prefixes. It's not in the scope of this change for sure.
richardkiss
force-pushed
the
ser_v2
branch
3 times, most recently
from
d241d3b
to
1b437ae
Compare
clvm/read_cache_lookup.py
Outdated
|
|
||
| def path_to_bytes(path: List[int]) -> bytes: | ||
| """ | ||
| Convert a list of 0/1 values to a path expected by clvm. |
There was a problem hiding this comment.
Small suggetion.
I think adding information showing that left=0 and right=1 is helpful.
Like
"""
Convert a list of 0(left)/1(right) values to a path expected by clvm.
...
"""
tests/test_object_cache.py
Outdated
| @@ -0,0 +1,41 @@ | |||
| import unittest | |||
There was a problem hiding this comment.
For consistency, test_object_cache.py should be renamed to object_cache_test.py.
Plus, how about adding read_cache_lookup_test.py?
Following lines to understand how ReadCacheLookup works was a real challenge for me without a testing.
For example, in my understanding root_hash of ReadCacheLookup is a tree-hash representation of read_stack but adding test by original author will greatly help understanding the intention of those code.
There was a problem hiding this comment.
I agree, this is difficult to understand and some tests may help elucidate (and test!).
There was a problem hiding this comment.
I've added some tests for the ReadCacheLookup class.
| # find BOTH minimal paths to `foo` | ||
| self.assertEqual( | ||
| rcl.find_paths(foo_hash, serialized_length=20), | ||
| set([bytes([4]), bytes([6])]), |
There was a problem hiding this comment.
PyCharm warns that set([a, b, c]) can be replaced with {a, b, c}
|
|
||
| class ObjectCacheTest(unittest.TestCase): | ||
| def check(self, obj_text, expected_hash, expected_length): | ||
| obj = assemble(obj_text) |
There was a problem hiding this comment.
I think circular dependency of clvm_tools should be avoided.
How about removing assemble and replacing with to_sexp_f here?
class ObjectCacheTest(unittest.TestCase):
def check(self, obj, expected_hash, expected_length):
th = ObjectCache(treehash)
self.assertEqual(th.get(obj).hex(), expected_hash)
sl = ObjectCache(serialized_length)
self.assertEqual(sl.get(obj), expected_length)
def test_various(self):
self.check(
# "0x00"
to_sexp_f(b'\x00'),
"47dc540c94ceb704a23875c11273e16bb0b8a87aed84de911f2133568115f254",
1,
)
self.check(
# "0"
to_sexp_f(None),
"4bf5122f344554c53bde2ebb8cd2b7e3d1600ad631c385a5d7cce23c7785459a", 1
)
self.check(
# "foo"
to_sexp_f("foo"),
"0080b50a51ecd0ccfaaa4d49dba866fe58724f18445d30202bafb03e21eef6cb", 4
)
self.check(
# "(foo . bar)",
to_sexp_f(("foo", "bar")),
"c518e45ae6a7b4146017b7a1d81639051b132f1f5572ce3088a3898a9ed1280b",
9,
)
self.check(
# "(this is a longer test of a deeper tree)"
to_sexp_f([b"this", b"is", b"\x02", b"longer", b"test", b"of", b"\x02", b"deeper", b"tree"]),
"0a072d7d860d77d8e290ced0fdb29a271198ca3db54d701c45d831e3aae6422c",
47,
)There was a problem hiding this comment.
The tests already depend upon clvm_tools as they use brun internally. I agree though, you're right that I should use to_sexp_f.
No description provided.