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

feat: reown on nano contracts #601

Merged
merged 3 commits into from
Nov 7, 2024
Merged

Conversation

andreabadesso
Copy link
Contributor

@andreabadesso andreabadesso commented Nov 7, 2024

Acceptance Criteria

  • We should disable reown when the network does not support nano contracts
  • We should add a missing key in allow-scripts

Security Checklist

  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

@andreabadesso andreabadesso force-pushed the feat/reown-on-nano-contracts branch from 9115c8e to e1bb3c7 Compare November 7, 2024 21:56
@andreabadesso andreabadesso self-assigned this Nov 7, 2024
@andreabadesso andreabadesso added the enhancement New feature or request label Nov 7, 2024
src/sagas/reown.js Show resolved Hide resolved
src/reducers/reducer.js Show resolved Hide resolved
src/actions.js Outdated Show resolved Hide resolved
src/sagas/reown.js Outdated Show resolved Hide resolved
@@ -130,12 +130,15 @@ function* isReownEnabled() {
}

function* init() {
const walletStartState = yield select((state) => state.walletStartState);

if (walletStartState !== WALLET_STATUS.READY) {
Copy link
Contributor

Choose a reason for hiding this comment

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

chore: We can remove WALLET_STATUS from the imports (line 75) since we removed the only places where it was being read.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks

@@ -254,10 +254,14 @@ export function* startWallet(action) {
yield put(setUniqueDeviceId(uniqueDeviceId));

try {
yield call(wallet.start.bind(wallet), {
// XXX: This comes as undefined when the facede is the wallet-service.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// XXX: This comes as undefined when the facede is the wallet-service.
// XXX: This comes as undefined when the facade is the wallet-service.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks

@andreabadesso andreabadesso force-pushed the feat/reown-on-nano-contracts branch from 25ba554 to 12d067c Compare November 7, 2024 22:44
@andreabadesso andreabadesso force-pushed the feat/reown-on-nano-contracts branch from 12d067c to bf83166 Compare November 7, 2024 22:49
@andreabadesso andreabadesso merged commit 122f808 into master Nov 7, 2024
1 check passed
@andreabadesso andreabadesso deleted the feat/reown-on-nano-contracts branch November 7, 2024 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants