refactor: monero-rs -> monero-oxide #836
Merged
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
|
Thanks a lot for taking this on! I'll review this soon. Over the holidays I'll have less time but I'll try to make this a priority. The general approach looks sound. One nitpick I have is that I'd like to have all the monero-oxide extension types (alternatives to monero-rs) moved into a single crate. Name would probably be monero-oxide-ext. We should also take a look at how Cuprate handles this. I'll leave more detailed actionable comments soon. |
Author
|
This does do this, I just called it |
binarybaron
reviewed
Dec 28, 2025
binarybaron
reviewed
Dec 28, 2025
binarybaron
reviewed
Dec 28, 2025
nabijaczleweli
force-pushed
the
monero-rs
branch
from
967984e to
11e48f1
Compare
|
In an abundance of caution, we should do a few manual tests to ensure backwards compatibility.
|
|
I'm also unsure if we should move the serde stuff in the new |
Author
|
All our serde implementations live in the same files as the types they correspond to. I leave the testing to you, I don't really know how to do those tests since I've never actually used this program for its intended purpose >_< |
|
|
They pass on master. |
|
I believe we may be working with a wrong view key somewhere. Bob is trying to scan a particular Monero transaction and he wants to verify that the correct amount is being transferred. This check fails. Bob also does not detect the transaction on chain but needs Alice to the transfer proof to him. He usually sees it on chain beforehand (especially in integration tests). |
nabijaczleweli
force-pushed
the
monero-rs
branch
from
11e48f1 to
313f715
Compare
what did you change to fix this? the force push makes it hard to understand |
Author
|
Nothing, just a clean rebase |
|
Compilations fails with: |
Author
|
Means it wants another rebase maybe? Those lines are wrong to my checkout... |
Author
|
No, it wants to be built on the latest version of the source branch where this is fixed (444b182). |
Did you push? |
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
binarybaron
reviewed
Jan 3, 2026
binarybaron
reviewed
Jan 3, 2026
| } | ||
|
|
||
| pub fn decompress(&self) -> Point { | ||
| self.point.decompress().expect("validated in constructor") |
There was a problem hiding this comment.
do we ever construct this type without the constructor?
binarybaron
reviewed
Jan 3, 2026
binarybaron
reviewed
Jan 3, 2026
binarybaron
reviewed
Jan 3, 2026
binarybaron
reviewed
Jan 3, 2026
| pub tx_keys: HashMap<monero::Address, monero::PrivateKey>, | ||
| /// | ||
| /// Key is `MoneroAddress::to_string()`. It's not viable to use the address directly. | ||
| pub tx_keys: HashMap<String, monero_oxide_ext::PrivateKey>, |
There was a problem hiding this comment.
Why is not viable? We can implement eq?
Author
There was a problem hiding this comment.
And MoneroAddress even implements PartialEq/Eq. But it doesn't implement Hash, and the type system is not good enough to let us work around this (I tried!).
There was a problem hiding this comment.
Got it. I opened an issue here monero-oxide/monero-oxide#158
binarybaron
reviewed
Jan 3, 2026
binarybaron
reviewed
Jan 3, 2026
binarybaron
reviewed
Jan 3, 2026
nabijaczleweli
force-pushed
the
monero-rs
branch
from
9a214da to
69b2e68
Compare
Author
|
Rebased, hopefully fixed? |
|
a44556a fixes it. Did you rebase on that one? |
|
Yeah, needs another rebase... |
|
bugbot run (cant hurt) |
nabijaczleweli
force-pushed
the
monero-rs
branch
from
69b2e68 to
0cbf20f
Compare
|
See the failed CI: |
… to monero-amount Ref: eigenwallet#764
This partially reverts commit 6008a35.
nabijaczleweli
force-pushed
the
monero-rs
branch
from
0cbf20f to
27b313e
Compare
|
Bounty of 0.6 XMR + 0.18 XMR paid to @nabijaczleweli. Thank you for the contribution. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Closes #764
Ref: #775:
Note
Monero stack migration to monero-oxide
monero-oxide-ext(Amount, PrivateKey/PublicKey, address serde) and adds it to workspace and UI typeshare generationmonerowithmonero-address/monero-oxide-wallettypes across crates (monero-sys,monero-wallet,monero-rpc-pool,swap-*, tests), updating APIs, serde, and network enumsmonero-sysFFI and public API to new types; adjusts transaction key handling (map keys now address strings), reserve proof, balances, transfers, and listenersWritten by Cursor Bugbot for commit 69b2e68. This will update automatically on new commits. Configure here.