Simplify the Transaction class

[a-noni-mousse]

High Level Overview of Change

It simplifies the Transaction class and removes unnecessary exception code and variables

Context of Change

Type of Change

• [ ] 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)
• [X] Refactor (non-breaking change that only restructures code)
• [ ] Tests (You added tests for code that already exists, or your new feature included in this PR)
• [ ] Documentation Updates
• [ ] Release

[a-noni-mousse]

I have fixed the issues and rebase the change to the develop.

[a-noni-mousse]

I have updated the code. please review!

[nbougalis]

I like this. If possible, consider whether the TransStatus and TxnSql enumerations can be combined. I think that TransStatus is only "internal" to the running binary, so it should be possible to just combine the two, keeping the values from TxnSql, and eliminating the various completely unused status codes or at least marking them as [[deprecated, maybe_unused]].

[a-noni-mousse]

I have made fixes you wanted @ximinez

[ximinez]

I like this. If possible, consider whether the TransStatus and TxnSql enumerations can be combined. I think that TransStatus is only "internal" to the running binary, so it should be possible to just combine the two, keeping the values from TxnSql, and eliminating the various completely unused status codes or at least marking them as [[deprecated, maybe_unused]].

@nbougalis This is a really cool idea. I did a quick check, and found that of the 9 TransStatus values, the first 6 map to TxnSql values, and of the other 3, only OBSOLETE is used. However, from what I can tell, even though it can be set to several different values, when the TransStatus is read, it's only compared against INCLUDED and INVALID. In other words, we could probably roll OBSOLETE into one of the other values, and get rid of TransStatus, but it would take some work that might be beyond the scope of this PR.

Edit: Oh, what the hell https://github.com/XRPLF/rippled/commit/78dcdc0aee991ddf64391e8d3e6541b4d81cdd85. Check out my review below for more details.

Edit 2: On second thought, I like this solution better https://github.com/XRPLF/rippled/commit/9201b263dafd8394a0602943d9d53bd670d2cf07.

[intelliot]

@a-noni-mousse - will you be able to consider (and possibly merge or cherry-pick) the commits that ximinez offered above?

[Bronek]

This looks like a bunch of unrelated changes in a single PR, not merged from develop for a long time.

We will close it now, but feel free to create a new PR on the current develop, ideally splitting this into separate PRs where changes do not depend on each other.