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

Adds 'should reject and not refund a pegout when a contract is trying… #156

Open
wants to merge 4 commits into
base: rits-refactors-9-2024-integration
Choose a base branch
from

Conversation

jeremy-then
Copy link
Contributor

@jeremy-then jeremy-then commented Oct 28, 2024

Adds 'should reject and not refund a pegout when a contract is trying to execute it' 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 15:53
@jeremy-then jeremy-then force-pushed the add-pegout-with-fee-above-value-test branch 2 times, most recently from f39a419 to f5065f6 Compare October 30, 2024 19:07
@jeremy-then jeremy-then force-pushed the add-pegout-request-from-contract-test branch from 11c5790 to 913b560 Compare October 30, 2024 20:20
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.

Very cool test

const deployCallReleaseBtcContract = async (rskTxHelper) => {

const address = await rskTxHelper.getClient().eth.personal.newAccount('');
await sendFromCow(rskTxHelper, address, Number(btcToWeis(0.5)));

Choose a reason for hiding this comment

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

I think we have a constant for 0.5 now? maybe I am wrong :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Updated.

Copy link

sonarcloud bot commented Oct 31, 2024

Base automatically changed from add-pegout-with-fee-above-value-test to rits-refactors-9-2024-integration October 31, 2024 15:06
Comment on lines +6 to +7
const TEST_RELEASE_BTC_CONTRACT = '../contracts/CallReleaseBtcContract.sol';
const TEST_RELEASE_BTC_CONTRACT_NAME = 'CallReleaseBtcContract';
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this a generic contract deployer perhaps these values should be received by param then?

);

return {
creatorAddress: address,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this valuable? Shouldn't we simply return the contract address?

const pegoutValueInSatoshis = MINIMUM_PEGOUT_AMOUNT_IN_SATOSHIS;

const { callReleaseBtcContract, creatorAddress } = await deployCallReleaseBtcContract(rskTxHelper);
const initialRskSenderBalanceInWeisBN = await rskTxHelper.getBalance(creatorAddress);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It doesn't have to be the contract creator the one that makes the call right? It can be any address


await assert2wpBalancesAfterPegoutFromContract(initial2wpBalances, pegoutValueInSatoshis);

// The rsk sender should lose the funds since there's no refund when a smart contract is trying to do a pegout
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 also be checking the contract address balance?

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