v2.5.0 staging to master #5193
Closed
v2.5.0 staging to master #5193
Conversation
This file contains 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
This PR changes it so if you're running the wallet inside of the node, instead of iterating the chain using the RPC system it'll use the chain directly. This is a large performance optimziation when scaanning the blockchain. Co-authored-by: Andrea <[email protected]>
* Introduce optional pagination to ChainProcessor * Added test
This version brings in optional elliptic curve point multiplication stats. Not enabled by default but useful for performance analysis.
…ncryption This commit adds an optional feature (that can be enabled only during local development) to print the number of cryptographic operations performed for note decryption and encryption. This feature is enabled by building `ironfish-rust-nodejs` through `yarn build:stats`. After doing that, and after starting `ironfish-cli` through `yarn start <command>`, it is possible to trigger the printing of the statistics by sending `SIGUSR1` to the CLI process. This commit has no impact on release builds of Iron Fish.
* Simplify ChainProcessor return logic We only ever cared if work was done, not that the specific hash was changed from the original. * Add test to ensure chain remains stable
makes it explicit that this should be the round 2 secret package
Running `wallet:rescan` when not connected to a node produces output like the following: ``` Scanning blocks: [░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░] 0% | 60 / 624032 | 0.00 / sec | ETA: N/AAdded block seq: 100, hash: 00000...bd9a1 Scanning blocks: [░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░] 0% | 128 / 624032 | 0.00 / sec | ETA: N/AAdded block seq: 200, hash: 00000...c61b7 Scanning blocks: [░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░] 0% | 282 / 624032 | 0.00 / sec | ETA: N/AAdded block seq: 300, hash: 00000...194db ``` That is: the progress bar output is disrupted by the scanner logs. This commit suppresses the logs coming from the wallet so that the progress bar is the only output displayed.
* Make unsignedTransaction flag visible in send command. * move display transaction summary above raw and unsigned transaction exits
…ool to the response class
`Job.abort()` sends a `JobAborted` request to the worker. `Worker.onMessageFromParent()` is able to handle such requests, but `Worker.parseRequest()` rejects them. Fixed `Worker.parseRequest()` so that such requests are no longer rejected. Also fixed `JobAbortedMessage.deserializePayload()` to return the correct type.
Perform decrypt operations in the background, fully using the worker threads, while blocks are fetched from the chain db in parallel.
Wallet scans get the blocks/transactions/notes from the chain db. The chain db contains data that has already been validated in the past (or that otherwise is assumed to be valid). For this reason, it should be possible to speed up scans by skipping expensive validation performed when parsing notes. More specifically, `NoteEncrypted` parses some elliptic curve points in compressed form. The regular parsing done through `jubjub::SubgroupPoint::from_bytes()` involves an expensive check: it checks whether the uncompressed point belongs to the prime-order subgroup. This check is perfromed using elliptic curve point multiplication, which is the most predominant and slowest operation happening during wallet scans. Due to this check, for each note, the wallet currently performs two elliptic curve point multiplications: one to parse the note, another to attempt the actual decryption. By using `from_bytes_unchecked()` instead of `from_bytes()` we can skip the first multiplication, effectively improving note decryption performance by a factor of *almost* 2x. This change only affects the performance of wallet scans. Note parsing and decryption outside of a scan is unaffected at the moment. In the future, we can disable validation in other contexts as we see fit.
This feature will be used to attach signatures to unsigned transactions. The main usecase is to allow the user to sign the transaction offline and then submit it to the network. The offline signing will happen on ledger devices.
This RPC is used to add signature to a transaction. It takes a transaction and a signature as input and returns a signed transaction.
In the case that the records to cleanup is very high, we shouldn't load all of them in memory before cleaning up. We should only load 1 leveldb memory block at most. This will ensure if there is a bad feedback loop this list won't permanently grow and continue to use more memory as it tries to clean up accounts. This also ensures that the account record deletion is considered a record so deleting thousands of empty account cleanup records doesn't run forever.
* adds ledger utility class * sorting package.json and using lastest library: * updating lock
Calling `Wallet.reset()` while a scanning is running can sometimes result in race conditions. In particular, `WalletScanner` may attempt to query account heads at the same time while accounts are being reset, resulting in errors. This commit fixes the problem by using database transactions (1) while `WalletScanner` read accounts and (2) while `Wallet.reset()` updates accounts. This commit also adds transactions in `Wallet.getEarliestHead()` and `Wallet.getLatestHead()`. These methods are not used during scanning, but they suffer from the same kind of problem. Note that this commit fixes specific race conditions related to getting the list of accounts to scan, but other race conditions that affect scanning may still exist.
* Add test for ArrayUtils * Improve shuffle test * Improve sample test
when started the Meter class sets an interval to update its underlying arrays. if the meter is not stopped then this interval will keep running and keep the meter in the heap and leaks the meter memory. the WalletScanner creates a new ScanState and a new Meter for each scan. when the wallet is at the head of the chain this creates a new scan each time a block is added. fixes the memory leak by ensuring that the ScanState Meter is stopped when the scan completes and its interval function is cleared
replaces 'getAccountByPublicAddress' with a more generic 'findAccount' that takes a predicate uses findAccount in postTransaction to find the account that has the same public address and is a spending account fixes issue where wallet has a spending account and a view only account with the same public address and cannot post a transaction
``` WARN: FromAsCasing: 'as' and 'FROM' keywords' casing do not match (line 1) ```
Send to the Docker build daemon the necessary files from the `.git` directory so that the build scripts can extract the hash of the HEAD commit.
When importing an account twice, it would not print out the name of the account with the duplicate spending key. This will now include that in the message. It also improves tests to check the error type instead of error message. We should avoid testing error messages in tests if there is a proper typed error to check.
So we can reuse this in the CLI and make this consistent everywhere.
So we can render these horizontal cards more easily
This new command shows more detailed information specifically about the blockchain than is shown by node status.
These were duplicated under the wrong file. Must have been due to a merge issue.
Also implements it for the `chain:blocks:info` command
This flag doesn't quite work as intended, but having missing flags can sometimes throw compilation errors in certain scenarios. It should throw errors in all scenarios, but that will be tackled soon.
This makes it so if you tell the RpcTransaction serializer not to include the serialized format, the notes will also not be included.
This converts a lot of our manually configured card layouts to use our new ui card element. This simplifies the formatting so we have more consistent, easier to use card formatting across the CLI.
* Update style guide to document command output info This should help users figure out how to design the output of their commands. * Update ironfish-cli/STYLE_GUIDE.md Co-authored-by: mat-if <[email protected]> --------- Co-authored-by: mat-if <[email protected]>
the WalletScanner uses a hash, 'scanFrom', for each account in its scanningAccounts list to check whether it should decrypt notes for the account. if scanFrom is null or equal to the previous block hash then notes are decrypted when connecting a block. scanFrom is set to the account head when calling refreshScanningAccounts and set to null when connecting a block if scanFrom is equal to the previous hash. however, when blocks are disconnected scanFrom is not currently changed. in the event of a reorg, decryption is skipped when connecting blocks from the fork if 'scanFrom' is still set to the hash of a disconnected block. regenerates fixtures for test of this behavior and updates disconnectBlock to set scanFrom to null for all accounts where it is equal to the disconnected block hash.
* ChainProcessor returns when head is not found * Added a test * Fixed lint
* Fix chain:rewind command to properly rewind the wallet * Fix chain:rewind
) * Add the ability for CLI commands to track and close an RPC client With the addition of the JSON changes, which requires us to await on the start command in the IronfishCommand, commands that used `this.sdk.connectRpc()` were hanging because the clients were not being closed. This change allows us to fetch a client from the IronfishCommand directly, which gives the IronfishCommand the ability to track the client, and close it when the code is done being ran/executed. * close connections in the finally block * remove unneeded return
This should make them not discoverable, but users can use them still.
…5174) This removes LocalFlags entirely, as well as removing these 3 flags from RemoteFlags.
…ing it as needed (#5175)
This makes chain:power standard and return JSON when you pass in --json flag and adds the --color flag support as well.
It already returned RpcTransaction with extra fields, so we can just use the serializer and attach these extra fields.
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
v2.5.0 staging to master
Testing Plan
Documentation
Does this change require any updates to the Iron Fish Docs (ex. the RPC API
Reference)? If yes, link a
related documentation pull request for the website.
Breaking Change
Is this a breaking change? If yes, add notes below on why this is breaking and label it with
breaking-change-rpcorbreaking-change-sdk.