search

mit-dci / opencbdc-tx

A transaction processor for a hypothetical, general-purpose, central bank digital currency
Other
896 stars 198 forks source link

Sentinel 2pc controller bug fixes #151

Closed mszulcz-mitre closed 1 year ago

mszulcz-mitre commented 2 years ago

This PR addresses the bugs identified in Issue #140 and Issue #141. The bug fixes and associated unit tests are the same as those described in the issues.

HalosGhost commented 2 years ago

@mszulcz-mitre this has a very minimal merge conflict with trunk (just the addition of some tests). When you have a moment and can rebase it, I'll give it one final review and we'll get this merged as well!

HalosGhost commented 2 years ago

@metalicjames when you have a chance to review this, I'd love a second set of eyes.

mszulcz-mitre commented 2 years ago

I modified the code to avoid the possibility of underflow. For clarity, I added another check to confirm that at least one sentinel endpoint is defined and I added a related unit test too.

mszulcz-mitre commented 1 year ago

@HalosGhost It looks like James approved the changes I made in response to his review. Would you like me to do anything more for this pull request?

HalosGhost commented 1 year ago

Sorry for the delay; I've finally had a chance to walk through the code locally and test it. Tested ACK. All these changes eliminate several failure points and do so minimally. Looks good to me.