refactor(common): allow custom default configs #6036
Closed
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
Description --- Removes redundant clones that were clogging up the lint trap with warnings. Motivation and Context --- Lint warnings are annoying! How Has This Been Tested? --- The linter is happy now. What process can a PR reviewer use to test or verify this change? --- Check the CI logs to ensure lint warnings are no longer present.
Description --- Updates the handling of the `ToRistrettoPoint` opcode. Closes tari-project#5818. Motivation and Context --- The `ToRistrettoPoint` opcode now requires that stack input be the canonical encoding of a Ristretto secret key. This PR updates the opcode documentation and adds a test for proper handling of invalid encoding. It also corrects the error returned on invalid stack input. There is a [separate PR](tari-project/rfcs#113) that updates the RFC documentation. How Has This Been Tested? --- Existing tests pass. A modified test passes. What process can a PR reviewer use to test or verify this change? --- Check that the updated documentation reflects the opcode handling. Check that the modified test correctly detects invalid input. Check that the error returned on invalid input is correct.
Description --- Fixes opcode signatures to mitigate security issues. Closes tari-project#5817. Motivation and Context --- Handling of signature-related opcodes introduces security issues related to signature forgery as described in tari-project#5817. This PR changes how signature-related message data is handled. Because of the new design, signature arithmetic support is removed. How Has This Been Tested? --- Existing tests pass or have been updated to reflect the new design. What process can a PR reviewer use to test or verify this change? --- Check that the new design matches the intent of the signature-related opcodes. Check that test modifications are correct. BREAKING CHANGE: Changes how some opcodes are processed, which renders some existing scripts and transactions invalid.
Description --- Adds versioning to all hash domains. Closes tari-project#5970. Motivation and Context --- Some hash function definitions rely on a default version number that is added during macro invocation. This isn't inherently a problem, but reduces clarity and could result in breaking changes if this default is ever changed. It seems prudent to add a specific version in all cases. How Has This Been Tested? --- Existing tests pass. What process can a PR reviewer use to test or verify this change? --- Confirm that the added version numbers correspond to the [default](https://github.com/tari-project/tari-crypto/blob/91d1e779e565dc816a54ab78368d44efcae78a23/src/hashing.rs#L616).
Description --- Added normal one-sided and stealth one-sided coinbase transactions. Changes are: - Coinbases can now only be paid to a nominated wallet address directly. - The minotari miner and merge mining proxy will only start if a valid minotari wallet address has been supplied for the particular network. - The miner and merge mining proxy does not depend on the wallet anymore to construct the coinbase; it is done directly. - Corresponding gRPC method (`rpc GetCoinbase (GetCoinbaseRequest) returns (GetCoinbaseResponse)`) has been removed from the wallet. - A memory-based transactions key manager has been created for use by the miner and merge mining proxy as they do not need persistent storage of keys. This also ensures that new entropy for the generation of keys is created each time the miner or merge mining proxy is started effectively negating any collisions of private keys. - All unused code in the output manager and transaction manager have been removed. Motivation and Context --- See tari-project#5930 How Has This Been Tested? --- Unit tests Cucumber tests System-level tests - _Completed_ What process can a PR reviewer use to test or verify this change? --- - Code walk-through - Run cucumber `Scenario: Get Transaction Info` and review the log files for `WALLET_A` where the stealth one-sided coinbases will be imported and where payment is made to `WALLET_B`. <!-- Checklist --> <!-- 1. Is the title of your PR in the form that would make nice release notes? The title, excluding the conventional commit tag, will be included exactly as is in the CHANGELOG, so please think about it carefully. --> Breaking Changes --- - [ ] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [x] Other - Please specify <!-- Does this include a breaking change? If so, include this line as a footer --> - Coinbases can now only be paid to a nominated wallet address directly. - Existing wallets need to be recovered into new wallets.
Description --- Enabled revealed-value proofs for all output types, which are be controlled via the consensus constants. Range-proof types are mapped to output types. From the merge mining proxy/miner log, coinbase output showing `range_proof_type: RevealedValue` and `Proof: None`:  _**Note:** The faucets changed due to `RangeProofType` now being parsed with snake case - `"range_proof_type":"RevealedValue"` changed to `"range_proof_type":"revealed_value"` on every output line._ Motivation and Context --- See tari-project#5968 How Has This Been Tested? --- Added unit tests Unit tests pass Cucumber tests pass System-level tests ~~[**TO BE COMPLETED**]~~ What process can a PR reviewer use to test or verify this change? --- Code walkthrough Review the additional unit tests Run system-level tests <!-- Checklist --> <!-- 1. Is the title of your PR in the form that would make nice release notes? The title, excluding the conventional commit tag, will be included exactly as is in the CHANGELOG, so please think about it carefully. --> Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify <!-- Does this include a breaking change? If so, include this line as a footer --> <!-- BREAKING CHANGE: Description what the user should do, e.g. delete a database, resync the chain -->
Description --- Fixed cucumber "Merge Mining" tests after merging the one-sided coinbase PR; I just fixed one simple thing but all three tests are now working fine. Motivation and Context --- The one-sided coinbase PR did not properly initialize the merge mining proxy. How Has This Been Tested? --- `cargo test --release --test cucumber -- --name "Merge Mining"` What process can a PR reviewer use to test or verify this change? --- `cargo test --release --test cucumber -- --name "Merge Mining"` <!-- Checklist --> <!-- 1. Is the title of your PR in the form that would make nice release notes? The title, excluding the conventional commit tag, will be included exactly as is in the CHANGELOG, so please think about it carefully. --> Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify <!-- Does this include a breaking change? If so, include this line as a footer --> <!-- BREAKING CHANGE: Description what the user should do, e.g. delete a database, resync the chain -->
Description --- Disabled the console wallet gRPC via a feature flag in the console wallet build configuration, where gRPC is not part of the default build. Trying to run with the console wallet with the config option `grpc_enabled = true` where the gRPC feature is not enabled for the build will result in a runtime error. Motivation and Context --- See tari-project#5930 How Has This Been Tested? --- Existing unit and cucumber tests pass What process can a PR reviewer use to test or verify this change? --- Code walkthrough <!-- Checklist --> <!-- 1. Is the title of your PR in the form that would make nice release notes? The title, excluding the conventional commit tag, will be included exactly as is in the CHANGELOG, so please think about it carefully. --> Breaking Changes --- - [ ] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [x] Other - The console wallet must be build with the `features = ["grpc"]` to enable gRPC <!-- Does this include a breaking change? If so, include this line as a footer --> <!-- BREAKING CHANGE: Description what the user should do, e.g. delete a database, resync the chain -->
Description --- Update snow Motivation and Context --- Commits: https://github.com/mcginty/snow/commits/v0.9.4 Non-breaking update to use the latest version Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify <!-- Does this include a breaking change? If so, include this line as a footer --> <!-- BREAKING CHANGE: Description what the user should do, e.g. delete a database, resync the chain -->
The MM proxy logs are completely blotted at the info level by instrumentation calls, rendering them effectively useless at the info level. This moves instrumentation down to DEBUG and TRACE levels, so they're still available, without spamming hundreds of MB in user-facing logs. What process can a PR reviewer use to test or verify this change? --- Logs will be much cleaner at INFO level now, <!-- Checklist --> <!-- 1. Is the title of your PR in the form that would make nice release notes? The title, excluding the conventional commit tag, will be included exactly as is in the CHANGELOG, so please think about it carefully. --> Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify <!-- Does this include a breaking change? If so, include this line as a footer --> <!-- BREAKING CHANGE: Description what the user should do, e.g. delete a database, resync the chain -->
Description --- Migrates all remaining uses of `lazy_static` to `once_cell` for better consistency with the rest of the ecosystem. Motivation and Context --- Other repositories in the ecosystem are moving from `lazy_static` to `once_cell` for better maintenance and to remove the use of a macro. This repository had a mix of both still in use, so this PR finishes moving everything over. The API is very similar, so changes were minimal. How Has This Been Tested? --- Existing tests pass. What process can a PR reviewer use to test or verify this change? --- Check the correctness of each change by asserting that each use of `Lazy` reflects the same functionality as the previous `lazy_static!` macro.
Description --- This adds aux chain support for merge mining monero Fixes: tari-project#5975 Motivation and Context --- This allows tari to be merged mined with multiple other chains, up to 253 other chains. See: https://github.com/SChernykh/p2pool/blob/merge-mining/docs/MERGE_MINING.MD#proposed-rpc-api How Has This Been Tested? ---
…lling (tari-project#5997) Description --- fix: always return correct value from is_triggered without first requiring the signal to be polled Motivation and Context --- OneshotSignal uses the `futures::Shared<Fut>` combinator to allow a single future to be cloned and awaited on in multiple places. Previously we used FuseFuture::is_terminated to determine if the signal is triggered. However the semantics of is_terminated and is_triggered are subtly different. is_terminated indicates whether a future should be polled again, which is true even if the trigger has fired. This PR uses an AtomicBool that is set to true at the time the trigger is fired so that polling first is not required. Thanks to @hansieodendaal for discovering this issue How Has This Been Tested? --- Updated existing test to check is_triggered, previously failed and now passes. What process can a PR reviewer use to test or verify this change? --- N/A Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify <!-- Does this include a breaking change? If so, include this line as a footer --> <!-- BREAKING CHANGE: Description what the user should do, e.g. delete a database, resync the chain -->
Description --- Add configuration and support for operating grpc over tls to enhance the overall security of communicating nodes, wallets, and miners. This includes a self-signed certificate generation function for using locally, although it's highly recommended to generate valid, and verifiable certificates if you plan on opening any service up to the internet. ~TODO (before merge, eta: 1day)~: Complete - ~Expand to the merge miner~ - ~Expand to the wallet~ - ~Display a warning about security when generating self signed certificates~ Motivation and Context --- Mo' security mo' better. Closes: tari-project#5808 How Has This Been Tested? --- Locally What process can a PR reviewer use to test or verify this change? --- Mostly read about how to set it up, and see if that makes sense. Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify
Description --- * Fix display panic in `base_layer/core/src/mempool/service/request.rs`. When a transaction without a kernel comes in, this will panic * Fix display to no longer call FixedHash to hex, as fixedhash already impl display.
Description --- Add a lazy static value that allows us to access what the current network is globally, so we can use the network byte to create network & domain-specific hashers. Motivation and Context --- This prevents the spending of duplicated UTXO's across networks as the hashes would compute differently based on the network byte. Closes: tari-project#5652 How Has This Been Tested? --- CI Breaking Changes --- - [ ] None - [x] Requires data directory on base node to be deleted - [x] Requires hard fork - [x] Other - Please specify This will invalidate all previous hashes on the network, and require both a network reset, with full data directory deletion.
Description --- Ups the minimum difficulty for stagenet. Motivation and Context --- See tari-project#5620 The numbers chosen represent 2 Apple M1 cpus mining at 100%. How Has This Been Tested? --- Manual
Description --- only store the keccak state and not the entire monero coinbase transaction Motivation and Context --- The monero coinbase transaction can be very large as it may contain multiple coinbases. We only store the keccak inner hash state with half of the hashed transaction. We hash the last part to verify its working. How Has This Been Tested? --- unit tests fixes: tari-project#5891
Description --- New faucet for esmeralda New gen blocks for esmeralda and nextnet How Has This Been Tested? --- unit tests
Description --- New release
Description Workflow for Windows installer file was incorrectly referencing the environment values to construct the installer filename. Motivation and Context Fix ```release assets``` filename for Windows installer. How Has This Been Tested? Built and checked in local fork. What process can a PR reviewer use to test or verify this change? Check ```Build the Windows installer``` workflow for the installer filename - ```tari_suite-1.0.0-pre.0-afb3c70-windows-x64-installer.exe``` <!-- Checklist --> <!-- 1. Is the title of your PR in the form that would make nice release notes? The title, excluding the conventional commit tag, will be included exactly as is in the CHANGELOG, so please think about it carefully. --> Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify <!-- Does this include a breaking change? If so, include this line as a footer --> <!-- BREAKING CHANGE: Description what the user should do, e.g. delete a database, resync the chain -->
Description --- This cleans up chat, and the contacts service of possible panic points. Instead managing Err results and attempting to propagate errors better. Motivation and Context --- Make the service friendlier, and more importantly don't cause panics that will crush the Mobile apps over ffi. How Has This Been Tested? --- Cucumber locally Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify
Description --- Upgrades dependencies version
Description --- Changes statuses of coinbase transactions and one-sided transactions Motivation and Context --- Transactions now show as one-sided or coinbase, not Faux anymore. This is more inline of what they are and shows more information to the user. How Has This Been Tested? --- manual + unit tests Breaking Changes --- Wallet database changes, and requires recovery to keep existing database.
Description --- Added some changes due to hazop findings. These include renaming for clarity, more accurate comments, and one improved database transaction for the blockchain backend to make removing headers atomic. Motivation and Context --- These changes were recommended as part of the ongoing hazop process. How Has This Been Tested? --- Existing tests pass What process can a PR reviewer use to test or verify this change? --- Code walk-through <!-- Checklist --> <!-- 1. Is the title of your PR in the form that would make nice release notes? The title, excluding the conventional commit tag, will be included exactly as is in the CHANGELOG, so please think about it carefully. --> Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify <!-- Does this include a breaking change? If so, include this line as a footer --> <!-- BREAKING CHANGE: Description what the user should do, e.g. delete a database, resync the chain -->
Description --- - Added wallet FFI shutdown tests to prove that all services have been terminated when the FFI `wallet_destroy` command has been run. - Added additional safeguards in the FFI `wallet_destroy` method to ensure the wallet has shut down. Motivation and Context --- See tari-project#5984 How Has This Been Tested? --- Added `pub fn test_wallet_shutdown()` in the wallet FFI library. What process can a PR reviewer use to test or verify this change? --- - Review test code - Run the test <!-- Checklist --> <!-- 1. Is the title of your PR in the form that would make nice release notes? The title, excluding the conventional commit tag, will be included exactly as is in the CHANGELOG, so please think about it carefully. --> Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify <!-- Does this include a breaking change? If so, include this line as a footer --> <!-- BREAKING CHANGE: Description what the user should do, e.g. delete a database, resync the chain -->
Description --- Added miner input processing to the minotari miner and merge mining proxy. The miner wallet address, base node gRPC address and basic gRPC connection will be verified. Motivation and Context --- Users did not have a good user experience See tari-project#5929 How Has This Been Tested? --- System-level testing What process can a PR reviewer use to test or verify this change? --- Code walk-through System-level testing <!-- Checklist --> <!-- 1. Is the title of your PR in the form that would make nice release notes? The title, excluding the conventional commit tag, will be included exactly as is in the CHANGELOG, so please think about it carefully. --> Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify <!-- Does this include a breaking change? If so, include this line as a footer --> <!-- BREAKING CHANGE: Description what the user should do, e.g. delete a database, resync the chain --> --------- Co-authored-by: SW van Heerden <[email protected]>
Description --- build fixes for display
tari-project#6021) Description Move cargo cross into a job and envs usage for single build command with env build options. Added job for local cross-compile on x86-64 for arm64 without cargo cross docker build. Added disabled job for cargo-auditables. Motivation and Context Use envs for different features and single build command, instead of duplicated build with different options. How Has This Been Tested? Built local fork. What process can a PR reviewer use to test or verify this change? Should see all jobs run successfully Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify
) Description --- Refactors the handling of network-based hashing operations introduced in tari-project#5980 to better handle read operations. Adds a sanity test for hash independence. Supersedes tari-project#6014. Closes tari-project#6003. Motivation and Context --- Recent work in tari-project#5980 binds the current network into consensus hashing. It uses a `Mutex`, which has two subtle issues. First, it allows the network to be set multiple times, which should not occur. Second, it locks on reads, which is unnecessary and inefficient. This PR updates how the current network is handled. It adds `Network::get_current_or_default` that will return either the current network (if it has been set) or the default network (if it has not). It adds `Network:set_current` that attempts to set the network; if it has been set before, this operation will fail. Note that calling `Network::get_current_or_default` does _not_ set the network for this purpose. It modifies the API for consensus hashing to add a wrapper that uses the current network. This wraps functionality allowing for specification of a network, which is useful for testing. On top of this new API, a new test is added that checks for distinct hashes using the same input but different networks. This is not comprehensive, but will detect obvious issues. How Has This Been Tested? --- Existing tests pass. A new test passes. What process can a PR reviewer use to test or verify this change? --- Check that the tests do what they claim. Check that the updates to consensus hashing properly introduce the expected wrapping functionality. Check that the updated network API does what it is supposed to.
Description --- Changes from hazop code review Tested manually
Description --- Findings from hazop meeting
Description --- On startup make the basenode ask you if you want to run mining or not. Add a flag `--enable-mining` Fix base node flag `--enable-grpc` Motivation and Context --- This will make it easier to mine from a 1-click perspective where a new miner does not need to edit confiles in order to mine How Has This Been Tested? --- manual
Description --- Updated the faucets and initial emission values for esmeralda, stagenet and nextnet. Note: The igor faucet has been uploaded but not activated. Motivation and Context --- Faucet values were not accounted for in the total emission. How Has This Been Tested? --- Existing unit tests pass. Updated unit test `fn esmeralda_schedule()`. Updated the esmeralda, stagenet and nextnet genesis blocks with new faucet MMR information. What process can a PR reviewer use to test or verify this change? --- Code walk-through Run the unit test <!-- Checklist --> <!-- 1. Is the title of your PR in the form that would make nice release notes? The title, excluding the conventional commit tag, will be included exactly as is in the CHANGELOG, so please think about it carefully. --> Breaking Changes --- - [ ] None - [ ] Requires data directory on base node to be deleted - [X] Requires hard fork - [ ] Other - Please specify <!-- Does this include a breaking change? If so, include this line as a footer --> BREAKING CHANGE: The block chain needs to be started from scratch
Description --- New release v1.0.0-pre.1
Description --- fixes the chat ffi build adds CI option to build chat ffi.
Description --- New release
Description --- Removed duplicate config settings in `b_peer_seeds.toml` introduced by a recent PR Motivation and Context --- See above How Has This Been Tested? --- Tested the clean `config.toml` creation with `cargo run --release --bin minotari_node` What process can a PR reviewer use to test or verify this change? --- Performa clean `cargo run --release --bin minotari_node` and review the created `config.toml` <!-- Checklist --> <!-- 1. Is the title of your PR in the form that would make nice release notes? The title, excluding the conventional commit tag, will be included exactly as is in the CHANGELOG, so please think about it carefully. --> Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify <!-- Does this include a breaking change? If so, include this line as a footer --> <!-- BREAKING CHANGE: Description what the user should do, e.g. delete a database, resync the chain -->
Description --- Added more faucet tests, updated constants Motivation and Context --- This should have been part of tari-project#6024 How Has This Been Tested? --- New unit tests What process can a PR reviewer use to test or verify this change? --- <!-- Checklist --> <!-- 1. Is the title of your PR in the form that would make nice release notes? The title, excluding the conventional commit tag, will be included exactly as is in the CHANGELOG, so please think about it carefully. --> Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify <!-- Does this include a breaking change? If so, include this line as a footer --> <!-- BREAKING CHANGE: Description what the user should do, e.g. delete a database, resync the chain -->
Description --- Cucumber tests are broken on dev atm Motivation and Context --- Fixes cucumber tests.
sdbondi
force-pushed
the
custom-default-configs
branch
from
196cf48 to
9040007
Compare
ghpbot-tari-project
added
P-acks_required
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.
Description
Adds
load_configuration_with_overridesthat exposes the functionality to load configs without including the prompting and custom templates.Motivation and Context
On the tari-dan repo, we want to customize the default configurations without having to PR to the tari repo. L2-related configs can be removed later in another PR.
How Has This Been Tested?
Lints pass, no breaking public interface changes
What process can a PR reviewer use to test or verify this change?
Base node default config works as before
Breaking Changes