Refactor lastSentCheque according to lastReceivedCheque

[holisticode]

PR #1590 introduced a lastReceivedCheque on the Peer, with corresponding methods to access this member.

However, for accessing the last sent cheque, we don't have this functionality.

For consistency, it would be better if we had this consistent, thus we should have a lastSentCheque with its accessor methods on Peer.

[mortelli]

Are we positive we only care about the last cheque, in all cases?

[vojtechsimetka]

Yes, that should be all we need as it is cumulative. The honey - crypto exchange rate is calculated upon receiving the cheque - at that point we have 2 last cheques which is sufficient.

[ralph-pichler]

The "fast" way to do it would be to just move the lastSentCheque to the Peer and call the load function in Swap where it currently uses the cheques map. But this will lead to calls from Swap to Peer so we'll have to add locking Peer (which we probably have to do anyway since we already access Peer from other Swap functions too).

I think a better approach would be to do a bigger refactoring and move all things related to a single peer away from Swap (including cheques and potentially balances) in one go.

My idea would be:

• functions like createCheque, sendCheque should all move to Peer. This will also have the advantage that we won't have to lock the entire Swap all the time. (e.g. currently we keep the entire Swap locked while waiting for the response of an EmitChequeMsg)
• functions like encodeCheque, sigHashCheque etc. should become package level functions. They are all pure functions and don't need to access anything from the struct Those functions are on Cheque now.
• the main point of Swap would be to interact with the stateStore, implement the Add function and handle stuff related to our own contract.
• Peer would get its own lock which would be held during the entirety of handleMsg and when Swap interacts with a Peer in Add
• Peer would copy a reference to the Oracle when created which will have its own lock. That way we don't have to lock the entire Swap just because a Peer needs to access the Oracle. (Also in the future you might use different Oracles with different Peers)

[holisticode]

Yes indeed I have also seen the need for a complete refactoring.

Swap.Add would become very slim and basically pass everything to Peer. The good thing about this is that the lock time in Swap becomes short, as the relevant stuff is locked inside Peer which are separate instances for each peer

[mortelli]

@ralph-pichler, is this issue going to address the following comment?

load and get are the wrong way round.
these accessors should be defined on the swapPeer.
loading balance, cheques from store should be done when the peer is initialised

p := s.getPeer(id)
p.lastSentCheque()
p.setLastSentCheque()
p.lastReceivedCheque()
p.setLastReceivedCheque()
p.balance()
p.setBalance()

Originally posted by @zelig in PR #1554

if not, please let me know so i can create a separate issue for this.

also: same goes for the issue of only accessing disk when writing values to the maps, instead of when reading them as well (we talked about this on friday with @holisticode)

thanks

[ralph-pichler]

@mortelli kind of. While not part of the issue description here it will all be changed as part of the same PR as the lastSentCheque refactor. (already started working on it today)