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

Mobile backend issues #20941

Closed
MariusVanDerWijden opened this issue Apr 17, 2020 · 8 comments · Fixed by #21596
Closed

Mobile backend issues #20941

MariusVanDerWijden opened this issue Apr 17, 2020 · 8 comments · Fixed by #21596

Comments

@MariusVanDerWijden
Copy link
Member

MariusVanDerWijden commented Apr 17, 2020

We are currently thinking about fixing some bugs in the mobile backend.
This issue shall act as an umbrella issue to collect some of the quirks of the backend that could be improved in the future. If you have any suggestions, feel free to add.

  • All functions are undocumented
    Maybe we can find a way to insert Javadoc into the generated.aar

  • Address.toString() returns {}Address
    In order to get the proper hex representation of the address, you need to call Address.getHex(). The method .toString() should return a proper printable representation of the object.

  • Hash.toString() returns {}Hash
    In order to get the proper representation you need to call Hash.getHex()

  • Block.toString() returns {}Block
    In order to get a serialized form of block, you need to call block.encodeJson()

  • Transaction.toString() returns {}Transaction
    In order to get a serialized form of a transaction, you need to call tx.encodeJson()

  • Header.toString() returns {}Header
    In order to get a serialized form of a header, you need to call header.encodeJson()

  • FoundationBootnodes() only returns mainnet bootnodes
    Currently developers are afaics not able to connect to the testnets without having to input a v5 enabled bootnode themselves. See params, cmd/utils, mobile: remove DiscoveryV5Bootnodes #20949

  • Remove the enodes type
    Type aliases for array types complicate the java code a lot. To add a enode to your config you have to run the following code.

Enode e = new Enode("enode://..:30303");
Enodes enodes = new Enodes(1);
enodes.set(0,e);
conf.setBootstrapNodes(enodes);

If we remove the enodes type and let setBootstrapNodes accept an array of Enode, it could look something like this:

Enode e = new Enode("enode://..:30303");
conf.setBootstrapNodes(Enode[]{e});
  • Detect panics in go-code and return an error
    Currently a lot of our code can panic, e.g. the reflection code used in mobile/interfaces.go. If a code panics, the resulting panic is not returned to the application, the application just stops working. This is not acceptable, we should write a small layer that checks for panics and returns an error if a panic occured.
@ligi
Copy link
Member

ligi commented Apr 17, 2020

for the javadoc I opened this one a while ago: #16127 - but it depends on #20922

@karalabe
Copy link
Member

Originally gomobile stripped out all documentation. We've collaborated a bit way back on the various code generators with gomobile and now when it binds the Go code, it also spits out a soures.jar. That should contain all the comments attached too, though we'd need to modify the uploads/builders to include it (no idea how that part works in the Java world).

@MariusVanDerWijden
Copy link
Member Author

We currently don't have the time to work on these issues. If someone wants to pick it up, please feel free! If you have any questions, you can ask here or ask me on gitter directly. It's really not a hard issue, most of it is essentially cleaning up the java api.

@sarathsomana
Copy link

@MariusVanDerWijden Does it also make sense to implement the Stringer interface for the above mentioned types ?

@MariusVanDerWijden
Copy link
Member Author

Yeah, I would say so. We are using golang's mobile package (https://github.com/golang/mobile) to generate the java library. If we implement the stringer interface, we should automatically get the .String() methods for the types.
That would make it way more idiomatic for java. Do you want to try?

@sarathsomana
Copy link

Yes. I'd love to try it.
EncodeJson method of Block, Header and Transaction return an error along with a string. what should the String() method return when it encounters an error ?

@MariusVanDerWijden
Copy link
Member Author

MariusVanDerWijden commented Aug 21, 2020

An error in golang is always a stringer, so you could return the string representation of the error. But I think that might not be the best idea. I would prefer, if you returned nil, because than the caller can test if block.String() != null in their java code.
Maybe @ligi could weigh in here?

@sarathsomana
Copy link

The method signature for String() doesn't allow to return nil.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants