Implementation of EthSwap mechanism

[rmoreliovlabs]

Description

This pull request is not intended to be merged, its purpose is to review the proof of contect of the EthSwap mechanism.

Motivation and Context

How Has This Been Tested?

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:

[fmacleal]

Very good job, I have a few suggestions to simplify some functions and ideas to avoid some changes that could be skipped.

Let me know your thoughts. 🙂

[rmoreliovlabs]

pipeline:run

[fmacleal]

Good refactor. 👍

[asoto-iov]

Added some comments, most of then related with the utility class but I'm still reviewing the PR.

[rmoreliovlabs]

pipeline:run

[rmoreliovlabs]

pipeline:run

[fmacleal]

Good job mate, it's a complex issue and I know you had to change it lots of time.
Congrats!
:)

[sonarcloud]

Quality Gate passed

Issues
75 New issues
0 Accepted issues

Measures
0 Security Hotspots
87.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[VaWheel]

may cause an issue if just skipped. should be analyzed in more detail.

[rmoreliovlabs]

The reason I used this approach is that it aligns with the existing logic in the BlockExecutor. Just like how invalid transactions are managed there, invalid Claim transactions are discarded if discardInvalidTxs is set to true. If not, the execution is stopped

For General Transactions, the handling is similar:

if (!acceptInvalidTransactions && !transactionExecuted) {
    if (discardInvalidTxs) {
        logger.warn("block: [{}] discarded tx: [{}]", block.getNumber(), tx.getHash());
        continue;
    } else {
        logger.warn("block: [{}] execution interrupted because of invalid tx: [{}]",
                    block.getNumber(), tx.getHash());
        profiler.stop(metric);
        return BlockResult.INTERRUPTED_EXECUTION_BLOCK_RESULT;
    }
}

Just as we discard invalid general transactions, we also discard invalid Claim transactions based on specific validation criteria. This ensures we maintain consistency in handling invalid transactions across the code
Let me know if you have any further questions!

[VaWheel]

But Claim transactions that don't have locked funds !claimTransactionValidator.hasLockedFunds(tx, track)) would be just discarded without interrupting the execution regardless of discardInvalidTxs ?

[rmoreliovlabs]

Yes, invalid claim transactions (those without locked funds) are indeed discarded without interrupting block execution, regardless of the discardInvalidTxs value. This behavior is intentional and aligned with the design outlined in the TDD and Sergio's proposal

The logic was implemented to ensure that claim transactions, which don't meet the requirement of having locked funds, are simply discarded to prevent them from affecting the overall block processing. This ensures that the chain execution continues smoothly without being interrupted by these invalid claim transactions. The proposal specifies that invalid claims should not stop the block execution but rather be skipped to maintain stability and performance. You can find the detailed reasoning behind this in the initial Sergio's proposal document here

Happy to explain further if needed!

[sonarcloud]

Quality Gate passed

Issues
51 New issues
0 Accepted issues

Measures
0 Security Hotspots
88.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud