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

Monero Wallet 0.11.0.0 #94

Closed
wants to merge 5 commits into from
Closed

Conversation

snaggen
Copy link

@snaggen snaggen commented Sep 11, 2017

This is a pullrequest for the new Monero Wallet. It is built against Freedesktop 1.6, the branch name for the application is "stable" since it is meant to be the stable version of the Monero Wallet.
It passes all the basic validations, but fails the strict validation since the screenshot is not the expected size since the existing screenshots was not of the desired size.

@snaggen
Copy link
Author

snaggen commented Sep 11, 2017

I realize that it might not be clear to everyone what monero is... it is a private crypto currency, think bitcoin but private. https://getmonero.org/
This flatpak is a re-packaging of their official 64 bit binary from their download section
https://getmonero.org/downloads/

@nedrichards
Copy link
Member

If this is an x86-64 only app you'll want a flathub.json file in there to make sure it only tries to build on that arch. You can copy the one from Slack, which works similarly.

@snaggen
Copy link
Author

snaggen commented Sep 12, 2017

Thanks for the pointer, added the flathub.json

@snaggen
Copy link
Author

snaggen commented Sep 12, 2017

Improved the appdata to make it pass strict validation

@TingPing
Copy link
Member

Test build: https://flathub.org/builds/#/buildrequests/2202?redirect_to_build=true

@barthalion
Copy link
Member

Scheduler another one here as buildbot wasn't interested in building the previous.

"*.la", "*.a"],
"modules": [
{
"name": "monero-core-gui",
Copy link
Member

Choose a reason for hiding this comment

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

Why not build it from source, given that it's open source?

Copy link
Author

Choose a reason for hiding this comment

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

That was my first intention (and still something I'm looking in to), but for some reason there is some QT-related segfaults when I run it in a flatpak. Not sure why, so for now I just settled with repackaging. But there are in fact a benefit of running a repackaged version, and that is that it is actually possible to verify that it is the exact same binary as released on getmonero.org. And since it is a wallet, it is important that no malicious code is involved. I have thought about creating a simple script that would download the official binary, and do sha comparison of the binary files in the flatpak and the officially released tar.gz file. Just so the users can feel safe.

Copy link
Member

Choose a reason for hiding this comment

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

It'd be ideal to support the other architectures on Flathub if possible - one of the big advantages Flathub can provide for app developers is ARM and ARM64 packaging.

Copy link
Member

Choose a reason for hiding this comment

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

I think you have that backwards, you can verify no malicious code is involved if you build it but downloading binaries you cannot. So I think the security argument is just wrong.

Adding to that the point @nedrichards made I believe there are only benefits to building it so we should look into those errors you are having.

Copy link
Author

Choose a reason for hiding this comment

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

Well, the trust is in the source as you say. But the users here are using binaries, so they have to trust flathub.org or getmonero.org. What I'm saysing is that Monero Core is already trusted by most users, hence it might be a point in providing their binaries.
Anyway, as I said, my original plan was to build from source. This re-wrap is just a way to get a flatpak out the door until I can figure out why things are not working.

Here is my repository for the source build:
https://github.com/snaggen/monero-wallet-flatpak/

Unfortunately I seem to have lost the latest work on that, where I used the 5.9 version of the KDE runtime, and built xmllistmodel from source also (since the one packaged there was built against another version of the KDE runtime, so it is no good). However, the xmllistmodel is a little tricky to build from source, or rather I used a "simple" build system to be able to manually copy the files to the correct direectory, since using the cmake will end up trying to install the libs on the read only file system.
If someone feels comfortable with debuging QT and KDE I can see if I can recreate my previous work, so you can continue where I got stuck.

Anyway, I think the re-wrapped version would be nice to have in the mean time.

@TingPing TingPing added the awaiting-changes Pull request waiting for inputs or changes from author label Sep 19, 2017
@snaggen
Copy link
Author

snaggen commented Sep 20, 2017

Here is the version of monero-wallet flatpak that builds from source
https://github.com/snaggen/monero-wallet-flatpak/

I have updated it against the KDE 5.9 sdk, it builds qtdeclarative from source aso.
It runs the first time letting you create a wallet. You then get to see that wallet and everything seems fine. However, the second time when you are going straight in to the wallet and not using the Wizard, it segfaults.
I haven't been able to build the wallet in debug mode, so the backtrace doesn't givet that much. So if anyone with QT/KDE debuggin skills want to give it a shot, just go ahead.

@snaggen
Copy link
Author

snaggen commented Sep 27, 2017

So, what is happening? Is there any Qt/Kde debugging experts that might help out, or should we go with the re-packaged version for now? Then maybe later, I can see if I can get the attention of the monero developers to assist me in debuging this...

@TingPing
Copy link
Member

Then maybe later, I can see if I can get the attention of the monero developers to assist me in debuging this...

Why delay, otherwise its easy to get to a point where it is never solved because shipping the binary was good enough.

@TingPing
Copy link
Member

So I built it and it seems like it is failing to get my password:

qml: Error opening wallet with empty password: invalid password

This failure then ends with another error which is the crash:

qml: closing wallet async : 41d7FXjswpK1111111111111111111111111111111111111111111111111111111111111111111111111111112KhNi4
~Wallet: Closing wallet
ASSERT failure in QCoreApplication::sendEvent: "Cannot send events to objects owned by a different thread. Current thread 0x0x353b230. Receiver '' (of type 'Wallet') was created in thread 0x0x10f12d0", file kernel/qcoreapplication.cpp, line 563

So that seems just like a bug on their part but It might be avoidable if it gets the password properly.

@snaggen
Copy link
Author

snaggen commented Sep 28, 2017

Well, that is about as far as I got the last time I really dug in to this if you look at the bug I filed in monero-core and when I reached out on https://www.reddit.com/r/Monero/comments/64c5pj/need_help_trying_to_build_a_flatpak_of_the_monero/
However, that time people didn't seem that eager to assist, hence my hesitation to reach out again. However, I have pinged them in the monero-core issue and asked for assistance there.

@snaggen
Copy link
Author

snaggen commented Sep 28, 2017

It seems that it is the call
r = epee::serialization::load_t_from_binary(m_account, account_data);
in wallet2.cpp that fails, due to
2017-09-28 19:12:52.612 7fb763bff700 ERROR net.http contrib/epee/include/storages/portable_storage.h:161 portable_storage: wrong binary format - signature mismatch

So, why does that happen in a flatpak built from source, but not repackaged....

@snaggen
Copy link
Author

snaggen commented Sep 29, 2017

The reason it doesn't crash here the first time is that the wizard doesn't use load_t_from_binary.

@snaggen
Copy link
Author

snaggen commented Sep 29, 2017

The root cause of this seems to be that the password is empty, causing the decryption to fail... bla bla bla.. throw exception.

Looking in main.qml I found the openWallet call and added debug there and verified that the password is empty in this case. However, I really can't figure out how the password is meant to be populated...
@fluffypony could you clarify this?

@TingPing TingPing added the blocked Pull requests that are blocked on something or won't be accepted in the currrent state label Oct 1, 2017
@TingPing
Copy link
Member

TingPing commented Jan 6, 2018

So I got a chance to try buildling it myself and everything seems to work fine:

{
  "app-id": "org.getmonero.Wallet",
  "runtime": "org.kde.Platform",
  "runtime-version": "5.9",
  "branch": "stable",
  "sdk": "org.kde.Sdk",
  "command": "monero-wallet-gui",
  "finish-args": [
      "--share=ipc",
      "--socket=x11",
      "--share=network",
      "--device=dri",
      "--filesystem=home"
  ],
  "modules": [
    {
      "name": "boost",
      "buildsystem": "simple",
      "cleanup": [
        "/include"
      ],
      "build-commands": [
        "./bootstrap.sh",
        "./b2 install variant=release link=shared runtime-link=shared --prefix=/app --with-system --with-date_time --with-filesystem --with-thread --with-serialization --with-program_options --with-chrono --with-regex -j `nproc`"
      ],
      "sources": [
        {
          "type": "archive",
          "url": "https://dl.bintray.com/boostorg/release/1.66.0/source/boost_1_66_0.tar.gz",
          "sha256": "bd0df411efd9a585e5a2212275f8762079fed8842264954675a4fddc46cfcf60"
        }
      ]
		},
    {
      "name": "monero-wallet",
      "buildsystem": "cmake-ninja",
      "config-opts": [
        "-DCMAKE_BUILD_TYPE=Release",
        "-DBUILD_DOCUMENTATION=OFF",
        "-DSTACK_TRACE=OFF",
        "-DBUILD_GUI_DEPS=ON",
        "-DINSTALL_VENDORED_LIBUNBOUND=ON"
      ],
      "cleanup": [
        "/lib",
        "/include"
      ],
      "sources": [
        {
          "type": "archive",
          "url": "https://github.com/monero-project/monero/archive/v0.11.1.0.tar.gz",
          "sha256": "b5b48d3e5317c599e1499278580e9a6ba3afc3536f4064fcf7b20840066a509b"
        }
      ]
    },
    {
      "name": "qtdeclarative",
      "buildsystem": "qmake",
      "no-make-install": true,
      "post-install": [
        "install -d /app/lib/qml/QtQuick",
        "cp -r qml/QtQuick/XmlListModel /app/lib/qml/QtQuick"
      ],
      "sources": [
        {
          "type": "archive",
          "url": "http://download.qt.io/archive/qt/5.9/5.9.3/submodules/qtdeclarative-opensource-src-5.9.3.tar.xz",
          "sha256": "505f66d2062c1d84ce743a0b4969531e1cf94e30970dc64efffe10f74f989407"
        }
      ]
    },
    {
      "name": "monero-gui",
      "buildsystem": "simple",
      "build-commands": [
        "echo 'var GUI_VERSION = \"0.11.1.0\";\nvar GUI_MONERO_VERSION = \"0.11.1.0\";' > version.js",
        "sed -i 's|/opt/$${TARGET}/bin|/app/bin|g' deployment.pri",
        "qmake CONFIG+='release libunwind_off' QMAKE_LFLAGS='-L/app/lib' PREFIX=/app",
        "make -C src/zxcvbn-c",
        "make -j `nproc` install",

        "for size in 16 24 32 48 64 96 128 256; do
          install -Dm644 images/appicons/${size}x${size}.png /app/share/icons/hicolor/${size}x${size}/apps/org.getmonero.Wallet.png
         done",
        "install -Dm644 org.getmonero.Wallet.appdata.xml /app/share/appdata/org.getmonero.Wallet.appdata.xml",
        "install -Dm644 org.getmonero.Wallet.desktop /app/share/applications/org.getmonero.Wallet.desktop"
      ],
      "sources": [
        {
          "type": "archive",
          "url": "https://github.com/monero-project/monero-gui/archive/v0.11.1.0.tar.gz",
          "sha256": "39870b40b81cfe986c4ccd379fdde2cf34dabe8e427f7a9723e73ec9ee4ceae0"
        },
        {
          "type": "file",
          "path": "org.getmonero.Wallet.appdata.xml"
        },
        {
          "type": "file",
          "path": "org.getmonero.Wallet.desktop"
        }
      ]
    }
  ]
}

@aleixpol @grulja Is there a reason that specific QtQuick widget isn't installed in the KDE runtime?

@TingPing TingPing removed the blocked Pull requests that are blocked on something or won't be accepted in the currrent state label Jan 6, 2018
@TingPing
Copy link
Member

TingPing commented Jan 6, 2018

@snaggen If you are still interested in working on this, things left on my wishlist (which might require patches):

  • Log directory tries to be /app/bin/ which is clearly broken
  • Default wallet should be in private directory not ~/Monero
  • Daemon logging directory should be in private directory not ~/.bitmonero

All of our changes should be proposed upstream also:

  • Appstream/Icons/Desktop File/Directories

@aleixpol
Copy link

aleixpol commented Jan 8, 2018

Is there a reason that specific QtQuick widget isn't installed in the KDE runtime?

QtQuick is definitely there. Can you tell me the error you are referring to?

@TingPing
Copy link
Member

TingPing commented Jan 8, 2018

QtQuick is definitely there. Can you tell me the error you are referring to?

It is just the single widget QtQuick/XmlListModel that is missing.

@snaggen Also I spoke too soon and do still get a crash.

@fluffypony
Copy link

@snaggen apologies, I never saw the GitHub mention in September - I get a LOT of GitHub notifications:) Best place to bounce this around is in #monero-gui or #monero-dev on Freenode.

We welcome upstream changes, perhaps the best way to provide flatpak-specific functionality would be via command-line arguments? eg. we already have a --log-file argument to change the log file location.

@snaggen
Copy link
Author

snaggen commented Jan 8, 2018

I will not be ablt to work on this for a while... and if you see my previous comment about the password, I really need help figuring this out.
As for upstreaming things, I could try to create pull requests for this, but I can't promise when that will be... things have to calm down a little bit first.

"Log directory tries to be /app/bin/ which is clearly broken" - yes this sounds wrong, and maybe we should try to use the --log-file @fluffypony is mentioning.

"Default wallet should be in private directory not ~/Monero" - I disagree, the wallet of the user should be where the user can find it and not in some hidden private folder.

"logging directory should be in private directory not ~/.bitmonero" - Agree, this should be changed.

@snaggen
Copy link
Author

snaggen commented Jul 5, 2018

Closing this since it is out of date and I will not be able to work on it.

@snaggen snaggen closed this Jul 5, 2018
fooishbar pushed a commit to fooishbar/flathub that referenced this pull request Apr 8, 2020
su-ex pushed a commit to su-ex/flathub that referenced this pull request Dec 9, 2020
Update riot-web_1.5.13_amd64.deb to 1.5.14
jbruechert pushed a commit to jbruechert/flathub that referenced this pull request Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-changes Pull request waiting for inputs or changes from author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants