Conversation
| public string DestinationAddress { get { return this.destinationAddress; } set { this.destinationAddress = value; } } | ||
|
|
||
| /// <summary>Chain on which STRAX minting or burning should occur.</summary> | ||
| public DestinationChain DestinationChain { get { return (DestinationChain)this.destinationChain; } set { this.destinationChain = (int)value; } } |
There was a problem hiding this comment.
If we are introducing this new field we will ideally need a migration mechanism for existing conversion requests stored by the multisig nodes.
There was a problem hiding this comment.
Given that we now have almost no conversion requests and that we can have multisig nodes upgraded in a few hours I think we can do without it at this stage.
But I agree, migration mechanism will be needed later on
| this.RetrievalType = retrievalType; | ||
| this.Amount = amount; | ||
| this.TargetAddress = targetAddress; | ||
| //this.TargetChain = targetChain; |
There was a problem hiding this comment.
Is this going to be introduced later?
There was a problem hiding this comment.
Yes. Adding it in the constructor breaks a lot of tests, I was about to work on it but had to switch to other task, so disabled it in this PR.
| /// blocks from another chain to look for cross chain deposit transactions. | ||
| /// </summary> | ||
| /// <remarks>Processes matured block deposits from the cirrus chain and creates instances of <see cref="ConversionRequest"/> which are | ||
| /// saved to <see cref="IConversionRequestRepository"/>.</remarks> |
There was a problem hiding this comment.
Bear in mind that the creation of ConversionRequests is something that was added onto this existing functionality. Conversion requests are obviously not used for normal cross-chain transfers.
Co-authored-by: zeptin <zeptin@gmail.com>
2 participants
This can be reviewed on commit by commit basis.
Mostly refactoring, comments, minor fixes, setup for future adding chains other than ETH for interoperability.