Refactor rlp

[ilanolkies]

Description

This pull request is a refactor and it adds minor bug fixing. It shouldn't change any existing functionality, modifying test cases was avoided.

Remove unused methods

Remove methods from RLP class that are not used like decode set or full traverse

Remove type detection

encode(Object input) was removed in favor of methods like encodeRskAddress that express the type of the element to be encoded. The overhead of detecting the type of the element, produced for example by creating Value class, is unnecessary since the developer needs to understand the protocol to know what element is decoded.

(See also https://github.com/rsksmart/rskj/security/code-scanning/2392 triggered in this PR)

Similarly, decode2 is removed in favor of decodeList. Note that the overhead produce for detecting if the element was a string or a list is visible calls like (RLPList) RLP.decode2(rlpEncoded).get(0);, two lists are created for decoding a single list.

The methods were moved to test utilities to prevent modifying the tests (RLPTestUtil)

Replace for constants

Constants hardcoded in the code are replaced for the respective static final constant. Also, duplicated constants were removed.

Remove decodeInt and fix byteArrayToInt

The method decodeInt is bugged. The data might not fit in the int. The method is removed in favor of ByteUtil.byteArrayToInt in FrameCoded that is used in other places.

The method decodeInt had the same bug, but is fixed by comparing to Integer.MAX_VALUE with intermediate BigInteger conversion. The conversion was already in the code so it doesn’t add significant overhead.

Fix toGas and byteArrayToLong

It follows from the considerations mentioned above. byteArrayToLong now favors only naturals big endian. toGas will return Long.MAX_VALUE on overflowing values, being gas limit natural number.

Motivation and Context

Researching on RLP now 👍

How Has This Been Tested?

Almost no new implementation was written, existent unit tests stand as source of trust. Test was added for fixed bug.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• Tests for the changes have been added (for bug fixes / features)
• Requires Activation Code (Hard Fork)

• Other information:
  fed:refactor-rlp

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

96.3% Coverage
0.0% Duplication

[rmoreliovlabs]

Just a NIT, I suggest you move these constants declaration to the beginning of the class.

[rmoreliovlabs]

LGTM

[Vovchyk]

pipeline:run

[Vovchyk]

@ilanolkies could you please rebase your PR? The pipeline fails and that could be due to a bit outdated version of master the pr is based on..

[Vovchyk]

pipeline:run

[Vovchyk]

@ilanolkies, seems the pipeline fails with on the powpeg jar building phase:

> Task :rskj:rskj-core:generateResources
> Task :rskj:rskj-core:jar

> Task :compileJava FAILED
/home/jenkins/workspace/Pipeline/powpeg-node/src/main/java/co/rsk/federate/Proof.java:31: error: cannot find symbol
        RLPList rlpList = (RLPList) RLP.decode2(rlpData).get(0);
                                       ^
  symbol:   method decode2(byte[])
  location: class org.ethereum.util.RLP
/home/jenkins/workspace/Pipeline/powpeg-node/src/main/java/co/rsk/federate/Proof.java:71: error: cannot find symbol
        RLPList rlpList = (RLPList)RLP.decode2(rlpData).get(0);

it looks like the powpeg code should be refactored as well according to your changes

[nagarev]

pipeline:run

[sonarcloud]

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

3 New issues
0 Security Hotspots
95.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[nagarev]

pipeline:run

[nagarev]

LGTM