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.

Sign up for GitHub

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

Jump to bottom

feat: added auxiliary callback to push base node state changes #5109 #5257

Merged
merged 16 commits into from
Apr 5, 2023
Merged

feat: added auxiliary callback to push base node state changes #5109 #5257

merged 16 commits into from
Apr 5, 2023

Conversation

agubarev
Copy link
Contributor

@agubarev agubarev commented Mar 17, 2023 •
edited
Loading

Description

Extra function argument to wallet_create FFI.

#5109

Motivation and Context

Giving mobile app more information about the active base node connection.

How Has This Been Tested?

@agubarev agubarev marked this pull request as draft March 17, 2023 13:57
@agubarev agubarev marked this pull request as ready for review March 20, 2023 12:28
SWvheerden
SWvheerden reviewed Mar 20, 2023
View reviewed changes
Copy link
Collaborator

@SWvheerden SWvheerden left a comment

Choose a reason for hiding this comment

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

This looks good so far. Just need to expand the pointer to include accessor functions for the data.

base_layer/wallet_ffi/src/callback_handler_tests.rs Show resolved Hide resolved
base_layer/wallet_ffi/src/lib.rs Outdated
@@ -135,6 +135,7 @@ use tari_utilities::{
SafePassword,
};
use tari_wallet::{
base_node_service::service::BaseNodeState,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to create a pub type TariBaseNodeState = base_node_service::service::BaseNodeState; for the .h file
Then we need to create accessor function for the FFI layer to be able to access the data inside if the pointer. Look at the other types.

@agubarev agubarev force-pushed the issue-5109-more-metadata-for-mobile branch from ba03436 to 293167f Compare March 27, 2023 08:19
SWvheerden
SWvheerden reviewed Mar 28, 2023
View reviewed changes
Copy link
Collaborator

@SWvheerden SWvheerden left a comment

Choose a reason for hiding this comment

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

looking on the right track here

base_layer/wallet_ffi/src/lib.rs Outdated
@@ -244,6 +244,40 @@ pub struct TariWallet {
shutdown: Shutdown,
}

#[derive(Debug)]
#[repr(C)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Look at the c code in the .h file we need accessor functions for the node id handling the Option<TariNodeId>
And the u128 is not support in C, its only fully supported in GCC c23 which had its draft published in Jan 2023.

@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Mar 28, 2023
stringhandler and others added 11 commits March 31, 2023 14:38
@stringhandler @agubarev
ci: trying to remove cancel in progress to improve mergequeue
b0b0692
@dependabot @agubarev
chore(deps): bump openssl from 0.10.45 to 0.10.48 (tari-project#5269)
3144dcd 
Bumps [openssl](https://github.com/sfackler/rust-openssl) from 0.10.45
to 0.10.48.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/sfackler/rust-openssl/releases">openssl's
releases</a>.</em></p>
<blockquote>
<h2>openssl v0.10.48</h2>
<h2>What's Changed</h2>
<ul>
<li>Fix LibreSSL version checking in openssl/ by <a
href="https://github.com/alex"><code>@​alex</code></a> in <a
href="https://redirect.github.com/sfackler/rust-openssl/pull/1851">sfackler/rust-openssl#1851</a></li>
<li>Skip a test that hangs on OpenSSL 3.1.0 by <a
href="https://github.com/alex"><code>@​alex</code></a> in <a
href="https://redirect.github.com/sfackler/rust-openssl/pull/1850">sfackler/rust-openssl#1850</a></li>
<li>Improve reliability of some tests by <a
href="https://github.com/smoelius"><code>@​smoelius</code></a> in <a
href="https://redirect.github.com/sfackler/rust-openssl/pull/1852">sfackler/rust-openssl#1852</a></li>
<li>Fix a series of security issues by <a
href="https://github.com/alex"><code>@​alex</code></a> in <a
href="https://redirect.github.com/sfackler/rust-openssl/pull/1854">sfackler/rust-openssl#1854</a></li>
<li>Release openssl v0.10.48 and openssl-sys v0.9.83 by <a
href="https://github.com/alex"><code>@​alex</code></a> in <a
href="https://redirect.github.com/sfackler/rust-openssl/pull/1855">sfackler/rust-openssl#1855</a></li>
</ul>
<h2>New Contributors</h2>
<ul>
<li><a href="https://github.com/smoelius"><code>@​smoelius</code></a>
made their first contribution in <a
href="https://redirect.github.com/sfackler/rust-openssl/pull/1852">sfackler/rust-openssl#1852</a></li>
</ul>
<p><strong>Full Changelog</strong>: <a
href="https://github.com/sfackler/rust-openssl/compare/openssl-v0.10.47...openssl-v0.10.48">https://github.com/sfackler/rust-openssl/compare/openssl-v0.10.47...openssl-v0.10.48</a></p>
<h2>openssl v0.10.47</h2>
<p>No release notes provided.</p>
<h2>openssl v0.10.46</h2>
<p>No release notes provided.</p>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/sfackler/rust-openssl/commit/4ff734fe4c5a22f7346b7b3c47ece4c4c1c01817"><code>4ff734f</code></a>
Release openssl v0.10.48 and openssl-sys v0.9.83 (<a
href="https://redirect.github.com/sfackler/rust-openssl/issues/1855">#1855</a>)</li>
<li><a
href="https://github.com/sfackler/rust-openssl/commit/5efceaabd69c540b487f6372be4982cf94884008"><code>5efceaa</code></a>
Merge pull request <a
href="https://redirect.github.com/sfackler/rust-openssl/issues/1854">#1854</a>
from alex/davids-openssl-of-horrors</li>
<li><a
href="https://github.com/sfackler/rust-openssl/commit/6ced4f305e44df7ca32e478621bf4840b122f1a3"><code>6ced4f3</code></a>
Fix race condition with X509Name creation</li>
<li><a
href="https://github.com/sfackler/rust-openssl/commit/a7528056c5be6f3fbabc52c2fd02882b208d5939"><code>a752805</code></a>
Document the horror show</li>
<li><a
href="https://github.com/sfackler/rust-openssl/commit/78aa9aa22cfd58ac33d1e19184cec667438fd2a1"><code>78aa9aa</code></a>
Always provide an X509V3Context in X509Extension::new because OpenSSL
require...</li>
<li><a
href="https://github.com/sfackler/rust-openssl/commit/332311b597cc444a10d4acaf122ee58bd1bc8ff8"><code>332311b</code></a>
Resolve an injection vulnerability in EKU creation</li>
<li><a
href="https://github.com/sfackler/rust-openssl/commit/482575bff434f58b80ffea34a9610d0ff265ac1f"><code>482575b</code></a>
Resolve an injection vulnerability in SAN creation</li>
<li><a
href="https://github.com/sfackler/rust-openssl/commit/690eeb2ac20d47e43f04d8cb43f03d4128946b81"><code>690eeb2</code></a>
Merge pull request <a
href="https://redirect.github.com/sfackler/rust-openssl/issues/1852">#1852</a>
from smoelius/master</li>
<li><a
href="https://github.com/sfackler/rust-openssl/commit/e5b6d97ed170f835b56440d79edcd46381a46ebc"><code>e5b6d97</code></a>
Improve reliability of some tests</li>
<li><a
href="https://github.com/sfackler/rust-openssl/commit/319200ab93e252a3c0e127adc1e4c43a90f063a1"><code>319200a</code></a>
Merge pull request <a
href="https://redirect.github.com/sfackler/rust-openssl/issues/1851">#1851</a>
from alex/libressl-versions</li>
<li>Additional commits viewable in <a
href="https://github.com/sfackler/rust-openssl/compare/openssl-v0.10.45...openssl-v0.10.48">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=openssl&package-manager=cargo&previous-version=0.10.45&new-version=0.10.48)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the
[Security Alerts
page](https://github.com/tari-project/tari/network/alerts).

</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@AaronFeickert @agubarev
fix: ensures mutable MMR bitmaps are compressed (tari-project#5278)
90f17b4 
Description
---
Ensures that mutable MMR root computation first performs deletion bitmap
compression.

Closes [issue 5277](tari-project#5277).

Motivation and Context
---
Currently, when mutable MMR roots [are
computed](https://github.com/tari-project/tari/blob/e334a404e432b0911bae3054a28d8e8ca5876e6c/base_layer/mmr/src/mutable_mmr.rs#L119-L131),
it's implicitly assumed that the underlying deletion bitmap has been
compressed. If it has not, it's possible for the resulting root to be
different than if the bitmap were compressed. This already resulted in
an intermittent [test
failure](tari-project#5268).

To reduce the risk that a caller does not perform this compression
correctly, this PR adds compression to the Merkle root computation
functionality directly. It also removes a few compression calls that
become redundant with this change.

Note that this may constitute an efficiency regression. If a mutable MMR
does not have any deletions since its last bitmap compression,
subsequent compression is not necessary. Further, we perform the
compression on a clone of the bitmap; this is necessary to avoid
mutability and ensure that operations like equality (which rely on root
computation) function properly. The efficiency consequences of this
change should be examined to ensure they are acceptable.

How Has This Been Tested?
---
Existing CI passes.

What process can a PR reviewer use to test or verify this change?
---
Run existing CI. To further check for intermittent test failures, loop
affected tests.

Breaking Changes
---

- [ ] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [x] Other - Please specify

It may be the case that this change affects the way mutable MMR roots
are computed in certain cases, which in turn may be a breaking change.
@leet4tari @agubarev
ci(fix): update miner filename (tari-project#5266)
1126254 
Description
update miner filename so release has unique filename per arch

Motivation and Context
currently release has only two miner files, which is broken as we should
have a file per arch.

How Has This Been Tested?
---
Built in local fork

What process can a PR reviewer use to test or verify this change?
---
Look at artifacts and see miner filenames match artifacts.

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 -->
@leet4tari @agubarev
ci(fix): libwallet update to support TestNet env (tari-project#5275)
5414d58 
Description
Updated the libwallet build to pass the TestNet env - ```TARI_NETWORK```
for tagged release

Motivation and Context
Split out the different networks when build on GitHub

How Has This Been Tested?
Build in local fork, tagged and untagged - compile completes

What process can a PR reviewer use to test or verify this change?
---


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 -->
@AaronFeickert @agubarev
feat: improved passphrase flow (tari-project#5279)
edac682 
Description
---
Improves the flow for setting or changing a passphrase.

Closes [issue 5127](tari-project#5127).

Motivation and Context
---
When setting or
[changing](tari-project#5175) a wallet
passphrase, the console wallet provides
[feedback](tari-project#5111) on the
strength of the provided passphrase. In the case of a weak passphrase,
it does not prompt the user to choose a better one.

This PR implements a better flow for this process, as shown in [this
flowchart](tari-project#5127 (comment)).

How Has This Been Tested?
---
Tested manually.

What process can a PR reviewer use to test or verify this change?
---
Testing needs to be done manually to assert that the process represented
by the linked flowchart is implemented. Manual testing should cover the
entire flow for these two operations:
- Setting the passphrase for a new wallet
- Changing the passphrase for an existing wallet

Breaking Changes
---
None.
@SWvheerden @stringhandler @agubarev
docs: update readme to include recommended versions (tari-project#5271)
86c89b1 
Description
---
Update readme to include recommended versions for each network.

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: stringhandler <stringhandler@gmail.com>
@agubarev
added FFI helper functions to extract individual values from TariBase…
a4ec602 
…NodeState
@agubarev
Merge branch 'development' into issue-5109-more-metadata-for-mobile
7ecc3db
@agubarev
cleanup
1282d75
@agubarev
Update ffi_basenode_state.rs
61db385
@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Apr 5, 2023
@SWvheerden SWvheerden merged commit b7f7d31 into tari-project:development Apr 5, 2023
@SWvheerden SWvheerden mentioned this pull request Apr 5, 2023
Merged
SWvheerden added a commit that referenced this pull request Apr 5, 2023
@SWvheerden
chore(version): v0.49.0-pre.6 (#5292)
b6a93ee 
##
[0.49.0-pre.6](v0.49.0-pre.5...v0.49.0-pre.6)
(2023-04-05)


### ⚠ BREAKING CHANGES

* move key manager service to key_manager (5284)
* add igor faucet (5281)
* reset dates for networks (5283)

### Features

* add igor faucet
([5281](#5281))
([bfc92fd](bfc92fd))
* added auxiliary callback to push base node state changes
[5109](#5109)
([#5257](#5257))
([b7f7d31](b7f7d31))
* move key manager service to key_manager
([5284](#5284))
([d50ed02](d50ed02))
* reset dates for networks
([5283](#5283))
([d6342a4](d6342a4))


### Bug Fixes

* resize transaction tab windows
([5290](#5290))
([bd95a85](bd95a85)),
closes [#4942](#4942)
[#5289](#5289)
[#12365](https://github.com/tari-project/tari/issues/12365)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SWvheerden SWvheerden SWvheerden approved these changes

Assignees
No one assigned
Labels
P-acks_required Process - Requires more ACKs or utACKs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@agubarev @SWvheerden @ghpbot-tari-project @stringhandler @AaronFeickert @leet4tari