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

Add pegout with weis rounded down to satoshis test #158

Conversation

jeremy-then
Copy link
Contributor

Add pegout with weis rounded down to satoshis test

@jeremy-then jeremy-then self-assigned this Oct 28, 2024
@jeremy-then jeremy-then requested a review from a team as a code owner October 28, 2024 17:01
Copy link

@apancorb apancorb left a comment

Choose a reason for hiding this comment

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

LGTM

lib/2wp-utils.js Outdated Show resolved Hide resolved
lib/tests/2wp.js Outdated Show resolved Hide resolved
@jeremy-then jeremy-then force-pushed the add-pegout-request-from-contract-test branch from 11c5790 to 913b560 Compare October 30, 2024 20:20
@jeremy-then jeremy-then force-pushed the add-pegout-with-weis-rounded-down-to-satoshis-test branch from b5984e2 to 4f507e4 Compare October 30, 2024 22:02
lib/constants.js Outdated
Comment on lines 127 to 129
const FEE_PER_KB_CHANGE_PK = '6a4b49312b91e203ddfb9bc2d900ebbd46fbede46a7462e770bedcb11ad405e9';
const FEE_PER_KB_CHANGE_ADDR = '53f8f6dabd612b6137215ddd7758bb5cdd638922';

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wasn't this previously added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

lib/tests/2wp.js Outdated
Comment on lines 860 to 863
const senderRecipientInfo = await createSenderRecipientInfo(rskTxHelper, btcTxHelper);
const peginValueInSatoshis = btcToSatoshis(0.5);
const btcPeginTxHash = await sendPegin(rskTxHelper, btcTxHelper, senderRecipientInfo.btcSenderAddressInfo, satoshisToBtc(peginValueInSatoshis));
await ensurePeginIsRegistered(rskTxHelper, btcPeginTxHash);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't there a new util function for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

lib/tests/2wp.js Outdated
Comment on lines 867 to 865
const pegoutValueInWeisBN = new BN('41234567891234560'); // 0.04123456789123456 in RBTC
const expectedPegoutValueInSatoshis = 4123456; // 0.04123456 in BTC
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we use the unit converter library to somehow make this clearer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It turns out, we cant. The unit converter rounds differently than the Bridge. The unit converter would yield 4123457, and the Bridge 4123456. So, I had to hardcode it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we update the unit converter library then?

Copy link

sonarqubecloud bot commented Nov 6, 2024

@jeremy-then jeremy-then merged commit 1308096 into add-pegout-request-from-contract-test Nov 6, 2024
2 of 3 checks passed
@jeremy-then jeremy-then deleted the add-pegout-with-weis-rounded-down-to-satoshis-test branch November 6, 2024 02:08
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.

4 participants