Refactor createNewAccount function to be an internal function and remove unused password argument #815
Conversation
This file contains hidden or 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
Upgrade GitHub Action checkout
Fix typos discovered by codespell
…ct-usage-removed fix: replaced AnyObject with Any
* develop-upstream: chore: EIP681 docs + more descriptive log message fix: ENS parsing requires chainID if an ENS address is expected to be requested/decoded - we do not assume that client is using Ethereum Mainnet! fix: RLP func encode for single value has named parameter chore: test refactoring - checking for nullability by using XCTAssertNotNil chore: replaced [Any]() with [] for default parameter value fix: removed as AnyObject from ENS related code chore: removed spacing in `" )` fix: spacings chore: removed redundant `parameters: []` in read operation fix: EIP681 - when parsing arguments chain ID must be defaulted to Mainnet instead of 0 fix: Networks.fromInt must not return optional fix: some spacing and docs fixed fix: ERC20BaseProperties are back to inherit from AnyObject (restricting protocol to classes) fix: removed the use of AnyObject in all places possible - replaced with Any # Conflicts: # Tests/web3swiftTests/localTests/ABIEncoderTest.swift # Tests/web3swiftTests/localTests/TransactionsTests.swift
Add Support For The Mnemonic Phrase To Be An Array
There was a problem hiding this comment.
LGTM! But possibly should be pointed to v4 branch? @JeneaVranceanu @yaroslavyaroslav
JeneaVranceanu previously approved these changes May 2, 2023
| @janndriessen You are right. I'll point that to v4. @sunyaojin Meanwhile please use |
| @sunyaojin I'll update develop-4.0 and maybe after that will ask you to merge develop-4.0 into your branch before we will merge this update. |
Merged
| Dear Lord, I wish to not to be the one who will solve this |
Comment on lines +109 to +110
| var transaction = transaction ?? defaultTransaction | ||
| transaction.to = resolver.resolverContractAddress |
There was a problem hiding this comment.
As a result of fixing merge conflicts, I've added a couple of variable renamings.
There was a problem hiding this comment.
From options to transaction.
JeneaVranceanu approved these changes May 3, 2023
JeneaVranceanu requested review from janndriessen, JeneaVranceanu and yaroslavyaroslav JeneaVranceanu approved these changes May 3, 2023
janndriessen approved these changes May 3, 2023
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.
Summary of Changes
This pull request refactors the
createNewAccountfunction to be aninternalfunction and removes thepasswordargument, which is never used in the function. The existing test cases have been run and all passed, showing that this change does not affect their functionality.
Test Data or Screenshots
By submitting this pull request, you are confirming the following:
developbranch.