Simplify store by moving code to base / other classes #290
Conversation
quantumagi
changed the title
|
Is this to go in before the release? |
|
It's not critical but it could speed up writing test cases. It's really hard to say. |
|
I think this change does a lot more than just simplifying the store and adds a bit of functionality so I'm keen to wait with it. |
|
No functionality was added - i.e. the code is doing nothing that it was not doing before. The PR splits the code up into digestible pieces (into new classes) which means code is also being moved around. However we can certainly wait with this. For instance, much complexity exists in the code relating to maintaining transactional integrity and leaving the store in a predictable state when errors occur. This is tricky due to the use of in-memory lookups that need to reflect the store state. To simplify things all of that code was moved into the |
|
@bokobza , can we take another look at this now? |
| /// <summary> | ||
| /// The purpose of this class is to restrict the operations that can be performed on the underlying | ||
| /// database - i.e. it provides a "higher level" layer to the underlying DBreeze transaction. | ||
| /// As such it provides a guarantees that any transient lookups will be kept in step with changes |
| namespace Stratis.FederatedPeg.Features.FederationGateway.TargetChain | ||
| { | ||
| /// <summary>Supported DBreeze transaction modes.</summary> | ||
| public enum CrossChainTransactionMode |
There was a problem hiding this comment.
If I correctly understand and we are only talking about transaction in the DB sense of the term, I think the name is a bit ambiguous here: CrossChainTransactionMode could be easily interpreted as different modes of performing a Cross chain transfer no?
There was a problem hiding this comment.
Changed to CrossChainDBTransactionMode.
See #281