aionnetwork / AVM

Enabling Java code to run in a blockchain environment
https://theoan.com/
MIT License
49 stars 25 forks source link

[CLOSED] toString/avm_toString disagreements #282

Closed aionbot closed 5 years ago

aionbot commented 5 years ago

Issue created by jeff-aion (on Tuesday Oct 16, 2018 at 21:50 GMT)

While testing an implementation of the #281 demo DApp, I noticed that user-provided toString() methods (avm_toString() after transformation) are being ignored in many cases, in favour of the literal toString().

Looking into this further I found that the String.avm_valueOf and StringBuilder.avm_append calls are relying too directly on their underlying standard JCL implementations. These implementations will call toString() on the given object, when converting it to a String instance.

For now, I will change these cases to call avm_toString(), directly, but I suspect that we may need toString() to call avm_toString() in all cases (although I am hesitant to make such a large change without further consideration) since there are probably other situations where things like this are happening. To long as we implement our toString, in shadow Object, to not expose any nondeterminism, problems here would be consistent behavioural bugs (not calling user's avm_toString()) as opposed to consensus-breaking non-determinism.

Additionally, while looking into testing this solution, I found that our invokedynamic string concatenation support also has some problems. We get a cast failure if we pass in a non-String to a sequence of " + " concatenations. Looking into this further, I think that the dynamic args to the CallSite (and concat implementation) should be Object[] instead of String[]. Beyond that, I suspect we can fix the toString() problem here by relying on our shadow StringBuilder so long as instantiating another shadow object seems appropriate within our implementation.