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.

Sign up for GitHub

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

Jump to bottom

Tok 388/forking tests #118

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Tok 388/forking tests #118

wants to merge 7 commits into from
+342 −4

Conversation

DinizSa
Copy link
Contributor

@DinizSa DinizSa commented Nov 21, 2024 •
edited
Loading

What

  • Forking tests

Why

  • Ensure smooth interactions with live components (ie. stRif interaction, migration)

Testing

  • use .env.31.fork env
  • run: bun run test:fork

Refs

jmendiola222
jmendiola222 reviewed Nov 22, 2024
View reviewed changes
package.json Outdated Show resolved Hide resolved
jmendiola222
jmendiola222 reviewed Nov 22, 2024
View reviewed changes
.env.30.fork Outdated Show resolved Hide resolved
jmendiola222
jmendiola222 reviewed Nov 22, 2024
View reviewed changes
.env.30.fork Outdated Show resolved Hide resolved
@DinizSa DinizSa force-pushed the tok-388/forking-tests branch 2 times, most recently from 09955f4 to 3adf7f2 Compare November 25, 2024 09:22
@DinizSa DinizSa marked this pull request as ready for review November 25, 2024 11:21
antomor
antomor reviewed Nov 25, 2024
View reviewed changes
package.json Outdated Show resolved Hide resolved
antomor
antomor approved these changes Nov 25, 2024
View reviewed changes
Copy link
Collaborator

@antomor antomor left a comment

Choose a reason for hiding this comment

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

@DinizSa Thank you, LGTM. Just a question: does the TOKEN_HOLDER_ADDRESS_FORK need to have funds? if so how much? and does it also need to have any particular permissions (i.e.: govenor, kycApprover, etc...)?

jmendiola222
jmendiola222 reviewed Nov 25, 2024
View reviewed changes
.env.30.fork Outdated Show resolved Hide resolved
@DinizSa
Copy link
Contributor Author

DinizSa commented Nov 25, 2024 •
edited
Loading

@antomor

does the TOKEN_HOLDER_ADDRESS_FORK need to have funds? if so how much? and does it also need to have any particular permissions (i.e.: govenor, kycApprover, etc...)?

The account needs to have RIF, stRIF and rbtc. Small amounts is ok: there is no hard coded amounts, since the test use the current balance of the tokens in the tests.

UPDATE: Pushed a commit using the foundation as the RIF and RBTC source for the distribution in the fork testing, only requiring that the respective tokens balances are greater than 0. I think this is a fair assumption, but let me know your thoughts.
This allowed to simplify the requirements for the TOKENS_HOLDER_ADDRESS_FORK , that is now renamed STRIF_HOLDER_ADDRESS_FORK

jmendiola222
jmendiola222 approved these changes Nov 25, 2024
View reviewed changes
Copy link
Collaborator

@jmendiola222 jmendiola222 left a comment

Choose a reason for hiding this comment

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

Good work, let's wait for the mainnet deployments and we should be covered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antomor antomor antomor approved these changes

@jmendiola222 jmendiola222 jmendiola222 approved these changes

@nmagariang nmagariang Awaiting requested review from nmagariang

@franciscotobar franciscotobar Awaiting requested review from franciscotobar

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DinizSa @jmendiola222 @antomor