Adds 'should do a basic legacy pegin with the value above minimum' test #92
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jeremy-then
force-pushed
the
add-legacy-pegin-test-above-minimum
branch
from
be67f49 to
a67efec
Compare
Base automatically changed from
create-new-2wp-file
to
rits-refactors-9-2024-integration
September 25, 2024 20:42
marcos-iov
reviewed
Sep 25, 2024
lib/tests/2wp.js
Outdated
Comment on lines
102
to
83
| const initialBridgeBalance = Number(await rskTxHelper.getBalance(BRIDGE_ADDRESS)); | ||
| const initialBridgeUtxosBalance = await getBridgeUtxosBalance(rskTxHelper); | ||
| const initialFederationAddressBalanceInSatoshis = await getBtcAddressBalanceInSatoshis(btcTxHelper, federationAddress); | ||
| const senderRecipientInfo = await createSenderRecipientInfo(rskTxHelper, btcTxHelper); | ||
| const initialSenderAddressBalanceInSatoshis = await getBtcAddressBalanceInSatoshis(btcTxHelper, senderRecipientInfo.btcSenderAddressInfo.address); |
There was a problem hiding this comment.
Since this is gonna get repeated quite a lot, pre and post peg-in/peg-out. What do you think of extracting it to a method? The method could return an object with the different balances
lib/tests/2wp.js
Outdated
Comment on lines
118
to
119
| const isBtcTxHashAlreadyProcessed = await bridge.methods.isBtcTxHashAlreadyProcessed(btcPeginTxHash).call(); | ||
| expect(isBtcTxHashAlreadyProcessed).to.be.true; |
There was a problem hiding this comment.
Should this maybe be part of ensurePeginIsRegistered?
There was a problem hiding this comment.
It depends. Somethings we want to assert that this is false. Inside ensurePeginIsRegistered it should always be true. But we can put it inside the ensurePeginIsRegistered and when needed, assert from the test that it is false.
lib/tests/2wp.js
Outdated
Comment on lines
122
to
126
| const recipient1RskAddressChecksumed = rskTxHelper.getClient().utils.toChecksumAddress(ensure0x(senderRecipientInfo.rskRecipientRskAddressInfo.address)); | ||
| const expectedEvent = createExpectedPeginBtcEvent(PEGIN_EVENTS.PEGIN_BTC, recipient1RskAddressChecksumed, btcPeginTxHash, peginValueInSatoshis); | ||
| const btcTxHashProcessedHeight = Number(await bridge.methods.getBtcTxHashProcessedHeight(btcPeginTxHash).call()); | ||
| const peginBtcEvent = await findEventInBlock(rskTxHelper, expectedEvent.name, btcTxHashProcessedHeight); | ||
| expect(peginBtcEvent).to.be.deep.equal(expectedEvent); |
There was a problem hiding this comment.
This could also go into it's own method, right?
jeremy-then
changed the base branch from
rits-refactors-9-2024-integration
to
refactor-federation-and-bridge-balances-check
jeremy-then
force-pushed
the
add-legacy-pegin-test-above-minimum
branch
2 times, most recently
from
dff4eb6 to
697367e
Compare
Base automatically changed from
refactor-federation-and-bridge-balances-check
to
rits-refactors-9-2024-integration
September 26, 2024 16:06
marcos-iov
reviewed
Sep 26, 2024
lib/tests/2wp.js
Outdated
| const senderRecipientInfo = await createSenderRecipientInfo(rskTxHelper, btcTxHelper); | ||
| const initialSenderAddressBalanceInSatoshis = await getBtcAddressBalanceInSatoshis(btcTxHelper, senderRecipientInfo.btcSenderAddressInfo.address); | ||
| // Value above minimum | ||
| const peginValueInSatoshis = minimumPeginValueInSatoshis + Number(btcToSatoshis(0.1)); |
There was a problem hiding this comment.
Suggested change
| const peginValueInSatoshis = minimumPeginValueInSatoshis + Number(btcToSatoshis(0.1)); | |
| const peginValueInSatoshis = minimumPeginValueInSatoshis + 1; |
Maybe make just above the minimum? To be sure
…nctions below tests.
jeremy-then
force-pushed
the
add-legacy-pegin-test-above-minimum
branch
from
697367e to
f700fbe
Compare
|
marcos-iov
approved these changes
Sep 26, 2024
marcos-iov
merged commit 24ffe3e
into
rits-refactors-9-2024-integration
4 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a basic legacy pegin test with a value above minimum.
Besides the value above minimum, this pegin does the same checks as this test in this pr: #87