Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes 2wp tests due to lovell changes. #208

Closed
wants to merge 6 commits into from

Conversation

jeremy-then
Copy link
Contributor

No description provided.

@jeremy-then jeremy-then self-assigned this Jan 24, 2025
@jeremy-then jeremy-then requested a review from a team as a code owner January 24, 2025 02:18
@@ -1197,30 +1187,33 @@ const assertReleaseBtcEvent = (releaseBtcEvent, releaseRequestedEvent) => {
expect(releaseBtcEvent.arguments.releaseRskTxHash).to.be.equal(releaseRequestedEvent.arguments.rskTxHash);
};

const assertSuccessfulPegoutEventsAreEmitted = async (pegoutsEvents, pegoutRequestReceivedTransactionHash, senderRecipientInfo, pegoutValueInSatoshis, bridgeStateAfterPegoutCreation) => {

const assertSuccessfulPegoutEventsAreEmitted = async (pegoutsEvents, pegoutRequestReceivedTransactionHash, senderRecipientInfo, pegoutValueInWeisBN, pegoutValueInSatoshis, bridgeStateBeforePegoutCreation, bridgeStateAfterPegoutCreation) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think is weird to receive both pegoutValueInWeis and pegoutValueInSatoshis. We could just receive the satoshis value and do the conversion to weis inside the method, wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I agree, but none of the conversion tools round it like the Bridge does. I haven't been able to convert the weis values to satoshis like the Bridge, so, I felt forced to hard code it here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh okey, i see.
what if we simplify the expectedPegoutValueInSatoshi? Is the same?

lib/tests/2wp.js Outdated Show resolved Hide resolved
@julia-zack julia-zack self-requested a review January 27, 2025 15:34
@jeremy-then jeremy-then force-pushed the add-migration-utxos-test branch from 9685358 to 8bfe343 Compare January 28, 2025 00:55
@jeremy-then jeremy-then marked this pull request as draft February 7, 2025 22:48
@jeremy-then jeremy-then closed this Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants