[venu-block1]

Change Description

Change Type

Select ONE

• Documentation

• Stability bug fix

• Other

• Other - special case

Consensus Changes

• Consensus Changes

API Changes

• API Changes

Documentation Additions

• Documentation Additions

[tbfleming]

This doesn't encode variant version changes correctly

[tbfleming]

• When creating the deltas, it's storing the newer (version 1) table_delta format, but writing version 0 into the stream
• The ABI file decodes version=0 mapping to table_delta_v1 instead of table_delta_v0
• The ABI file is missing row_v0 and table_delta_v0

[venu-block1]

• The ABI file is missing row_v0 and table_delta_v0

May be I'm misunderstanding the not backward compatible part of this comment "... its a simple change but it is not strictly speaking backwards compatible" on the slack discussion thread. Now, I have both table_delta_v0 and table_delta_v1 in ABI file.

[tbfleming]

Later on in that thread we decided to go with strict backwards compatibility.

[venu-block1]

@tbfleming, I made changes in commit f447093 so that both table_delta_v0 and table_delta_v1 are in ABI file.

Can you please review and confirm if I covered all the changes that are required to be done for this issue? Are there any other code changes I should be doing to advance this draft PR to PR? Thanks!

[tbfleming]

This still needs fixing:

• When creating the deltas, it's storing the newer (version 1) table_delta format, but writing version 0 into the stream

[venu-block1]

This still needs fixing:

• When creating the deltas, it's storing the newer (version 1) table_delta format, but writing version 0 into the stream

Please review commit f57011c, and confirm the change fixes this. Thanks!

[tbfleming]

Approved. Switch it from draft mode.

[victorj8]

I noticed we currently don't have a unit test, for the case where we are updating a value (1), if you could add some that would be great, specially to test_deltas_contract and/or test_deltas_kv (which is where rocksdb and chainbase would be used for delta calculation)

[venu-block1]

I noticed we currently don't have a unit test, for the case where we are updating a value (1), if you could add some that would be great, specially to test_deltas_contract and/or test_deltas_kv (which is where rocksdb and chainbase would be used for delta calculation)

Changes to exiting test cases to cover present values (0 and 2) are made in b0cf772. Created a ticket to cover present value 1.