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

Fix undefined meta transaction type for SCW #68

Merged
merged 6 commits into from
Mar 3, 2021

Conversation

artsparkAI
Copy link
Contributor

@artsparkAI artsparkAI commented Feb 25, 2021

Quick fix for #67
Biconomy sdk inits fine after this check

It seems that wherever smartContractMetaTransactionMap is used for SCW it has an address from api, so moving it inside the branch seems fine.

Built files NOT included here because my particular node version was causing a huge diff in dist/mexa.js, someone on the team will have to build probably.

Copy link
Contributor

@livingrock7 livingrock7 left a comment

Choose a reason for hiding this comment

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

Looks good. Will test locally and update along with build files.

@artsparkAI artsparkAI changed the base branch from master to integration February 25, 2021 16:01
@artsparkAI
Copy link
Contributor Author

Changed to integration branch.

By the way, I'm not certain where the difference in build comes from, but I suspect it's node version. If you add an engines field to your package.json it indicates to developers which version to use. It's informational metadata only, it doesn't actually change anything about the build.
https://docs.npmjs.com/cli/v7/configuring-npm/package-json#engines

@artsparkAI artsparkAI changed the title Move metaTxMap to inside of branch Fix undefined meta transaction type for SCW Feb 25, 2021
@artsparkAI
Copy link
Contributor Author

I was incorrect in my assumption, api.contractAddr doesn't include anything for the SCW. Had to make changes to use the config.SCW for smart contract wallet.

Copy link
Contributor

@livingrock7 livingrock7 left a comment

Choose a reason for hiding this comment

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

can you confirm based on below review commen please L307

src/Biconomy.js Outdated Show resolved Hide resolved
@livingrock7 livingrock7 merged commit 0090fe8 into bcnmy:integration Mar 3, 2021
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.

3 participants