Java Borsh implementation | Bounty $3000 in NEAR

[ilblackdragon]

Description

A Java library that allows to generically serialize and deserialize Borsh

Context

Borsh is binary serializer that NEAR uses. Java developers may want to use it to interact with NEAR. Borsh has a simple specification outlined at https://borsh.io

Also Rust and JS implementations can be found here: https://github.com/near/borsh

Ideally Java implementation should follow interface similar to Rust, where an existing class can be just serialized / deserialized without any extra code.

Acceptance Criteria

• [x] A borshj library that other projects can include to use Borsh
• [x] Any "enabled" struct can be serialized with myobj.borshSerialize() and MyClass.borshDeserialize(bytes) (or simiilar)
• [ ] Test coverage for the spec similar to Rust tests - https://github.com/near/borsh/tree/master/borsh-rs/borsh/tests
• [x] Fuzz testing to make sure there are no edge cases

Bounty

$3000 in NEAR

[artob]

As discussed on a call with @potatodepaulo just now, I'm working on this bounty, expecting to deliver by end of next week.

[artob]

My in-the-works project is now public at github.com/artob/borshj. It's almost finished, I will wrap it up in the next days. Will have a call with @potatodepaulo about it shortly.

[behaviary]

@ailisp will review this code. @artob please let him know when you feel it's ready for a review.

[ailisp]

Looks it's overall finished and in good shipped. I reviewed existing code and here is something i found:

• String UTF-16 - looks good

• consider code reuse between BorshBuffer<>BorshReader BorshWriter<>BorshBuffer

• some TODO, like serialize array

• reader and writer test not completed

• consider add a recursive implements Borsh test cases, e.g.:

  
  public class Point implements Borsh {}                                                                         
  public class Rect implements Borsh {                                                                           
  public Point topLeft;                                                                                      
  public Point bottomRight;                                                                                  

}

And test Rect can also be serialized and deserialized                                                          

[artob]

@ailisp Thanks for the (slightly premature, but appreciated!) review :) I will address your comments and push a new version for review in the next day or two.

Do you have any thoughts regarding what we should do for fuzz testing here? That is one of the listed acceptance criteria, but it wasn't obvious to me how to proceed on that, since decoding random bytes (such as produced by fuzzing) would need a schema to make sense of them.

[MaksymZavershynskyi]

You could use Java reflection to generate some random hierarchies of classes, then serialize them, deserialize the result and compare that it is the same as before serialization.

[artob]

@ailisp @nearmax All right, I've pushed a new version that I believe is ready for your substantive review and does address all previous comments as follows:

consider code reuse between BorshBuffer<>BorshReader BorshWriter<>BorshBuffer

I've refactored the implementation, moving common code into the new BorshInput and BorshOutput interfaces.

This way, the implementations of the user-facing BorshBuffer, BorshReader, and BorshWriter classes are focused only on their respective glue points (to NIO buffers and I/O streams, respectively).

And the easiest-to-use interaction points remain the Borsh.serialize() and Borsh.deserialize() static methods.

some TODO, like serialize array

I've implemented array serialization and deserialization now. The only TODO comments left in the code base now are to note where there might be optimization opportunities (no change in functionality) to pursue going forward.

reader and writer test not completed

I've added elementary BorshReader tests and BorshWriter tests.

That's probably sufficient, these being standard Java interfaces and what with the implementation of these classes being now pretty trivial, since all serialization code is shared with BorshBuffer and extensively exercised in the BorshBuffer tests.

consider add a recursive implements Borsh test cases, e.g.:

Thanks, I've added exactly this test case. And that all works fine.

Overall, we're now at more than functional parity with the Borsh implementation for TypeScript, so I believe the only substantial thing left to do is the fuzz testing (I'm working on that along the lines of @nearmax's suggest). Please do provide feedback if that's not the case.

[artob]

@ailisp @nearmax I've now implemented fuzz testing as well, based on @nearmax's suggestion. Using Java reflection plus cglib, we generate a random hierarchy of recursive Java classes, and roundtrip (using Borsh.serialize() followed by Borsh.deserialize()) instances thereof to find any edge cases. Looking good at high levels of iterations and deep nesting.

[MaksymZavershynskyi]

@ailisp Could you please take a look?

[ailisp]

@artob @nearmax I took a look, all comments are addressed and fuzzy test is done. The repo satisfies all of acceptance criteria and I think it's a beautiful implementation that can be a reference for borsh in other programming languages :+1:

Some additional improvements:

• zero size corner cases is missing, i think this is the only missing one: https://github.com/near/borsh/blob/master/borsh-rs/borsh/tests/test_zero_size.rs

• add a enum test

Sorry i was wrong on java enum, it still deserves a test.

[artob]

Thanks, @ailisp, I appreciate the review and the kind words. I'll add the additional improvements you mentioned as well :+1:

[artob]

We've moved the BorshJ repository to near/borshj and I've ticketed @ailisp's follow-up comments at https://github.com/near/borshj/issues/1.

@potatodepaulo Since this bounty is completed and has been paid out, you could go ahead and close this?