Fix the issue of Nil/Zero-length-byte-array value

[manish-sethi]

Signed-off-by: manish manish.sethi@gmail.com

Type of change

• Bug fix

Description

• The zero length byte array value from a node chaincode reaches the fabric as is, instead of as a nil as in the case of a chaincode in golang/java. This prevents current fabric code to set the delete marker in the write-set. This patch fixes this issue.
• During commit, the write-set that has a value nil/zero-lengh byte array is interpreted as delete.

Additional details

The above issue is caused by the fact that unlike golanag and java chaincode shim, the protobuf library used by node shim is
not google-protobuf but protobufjs which has a different behavior for the default values.

Related issues

FAB-18386

[denyeart]

Logic represents the discussed design.
Tests look comprehensive.
I also tested e2e. Reproduced the issue with node chaincode, verified that a patched peer successfully converts the empty byte array to a delete during the commit.
Nice work!

[yacovm]

I'm fairly ignorant about the ledger code base, but, looking into the code that is not part of this change set:

1. IsDelete() here still looks only at nil. Is that a problem?
2. KVWrite.IsDelete here only looks at if the value is nil. Is that a problem?
3. Same thing for here. Is that a problem?

[manish-sethi]

1. IsDelete() here still looks only at nil. Is that a problem?

No, that's not a problem. This is the case when actually a Delete() is invoked on the UpdateBatch while preparing the batch (for committing) here. I realized while fixing this bug that it will be better to make this explicit by introducing a bool field here but then decided to limit the changes in this PR to this fix and plan to enhance the readability of this specific portion separately in master (and perhaps backport to 2.2 only).

2. KVWrite.IsDelete here only looks at if the value is nil. Is that a problem?

At this place the input is the same update batch that is prepared at the location mentioned in the (1) above (i.e., the in-memory value is not set, if the operation is a Delete). The object of type KVWrite that is prepared here by that input is for the purpose of computing the serialized form of the update batch, so we can compute the commit hash over the updates that are being applied to the statedb.

3. Same thing for here. Is that a problem?

Same as in 2, the same update batch is used to construct the object that is fed into the listeners that are interested in the state updates (such as couchdb indexes creator and the discovery code).

In summary, the source of all these three is the field in the VersionedValue inside the UpdateBatch, in which the field Value is not set, if it represents a delete operation for the corresponding key, in the batch.