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

Monero CI failure #351

Closed
kayabaNerve opened this issue Aug 21, 2023 · 9 comments
Closed

Monero CI failure #351

kayabaNerve opened this issue Aug 21, 2023 · 9 comments
Labels
critical This is critical monero An issue with the Monero library/integration

Comments

@kayabaNerve
Copy link
Member

https://github.com/serai-dex/serai/actions/runs/5923190999/job/16058367418

serai/tests/processor/src/lib.rs

Line 287 in 34397b3

(hash, rpc.get_block(hash).await.unwrap().serialize())
returned invalid node.

This can only happen if:

  1. The Monero response wasn't UTF-8.
  2. It was an error variant (which it shouldn't be since we're getting a block it gave us the hash for).
  3. Our parsing code has an error.
  4. Our hash code has an error.

Unfortunately, we do not have the granularity to know what the answer is.
@kayabaNerve kayabaNerve added critical This is critical monero An issue with the Monero library/integration labels Aug 21, 2023
@kayabaNerve kayabaNerve mentioned this issue Aug 21, 2023
Closed
@kayabaNerve
Copy link
Member Author

https://github.com/serai-dex/serai/actions/runs/6378881560/job/17310298162?pr=388, same spot.

@kayabaNerve kayabaNerve mentioned this issue Oct 3, 2023
Closed
@kayabaNerve
Copy link
Member Author

https://github.com/serai-dex/serai/actions/runs/6377392652/job/17305972503 same spot, different error. Since it was ConnectionError, I initially thought the node simply hadn't booted before the test ran, but no, prior calls executed.

Maybe it's some issue with long-lived Monero connections? If I run the binary to reserialize the blockchain for long enough, I eventually get similar behavior...

@Boog900
Copy link
Contributor

Boog900 commented Oct 4, 2023

I'm getting a similar issue in Cuprate every now and then I get error requesting a block or transaction, no other RPC request is giving me issues. I don't think it's because of long-lived connections as I was holding the connections for a while without requesting any blocks with no issues but when I start requesting blocks/ txs I start getting issues.

I did a bit of investigating exposed some errors and found that here keeps erroring with expected value at line 1 column 1

At the moment I have no clue why

@Boog900
Copy link
Contributor

Boog900 commented Oct 4, 2023

well I found my issue:

<html>
<head><title>429 Too Many Requests</title></head>
<body>
<center><h1>429 Too Many Requests</h1></center>
<hr><center>nginx</center>
</body>
</html>

guessing this is unrelated to the orignal issue

@kayabaNerve
Copy link
Member Author

If it's from nginx, yeah, it's unrelated to the CI failures. I wouldn't be surprised if the Monero node has some similar bs causing out own ConnectionError/InvalidNode reports though.

Our RPC code really needs a large rewrite re: error handling...

@kayabaNerve
Copy link
Member Author

I moved the processor tests from expecting a local Monero node to be spawned to using a dockertest setup as part of #405.

I've gotten the following list of errors, from a single run of the processor tests, after doing so:

[coins/monero/src/rpc/mod.rs:150] (raw, e) = (
    "<html><head><title>Unauthorized Access</title></head><body><h1>401 Unauthorized</h1></body></html>",
    Error("expected value", line: 1, column: 1),
)
thread 'tokio-runtime-worker' panicked at processor/src/networks/monero.rs:637:8:
called `Result::unwrap()` on an `Err` value: InvalidNode
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
[coins/monero/src/rpc/mod.rs:502] e = Custom {
    kind: Other,
    error: "invalid header",
}
[coins/monero/src/rpc/mod.rs:150] (raw, e) = (
    "<html><head><title>Unauthorized Access</title></head><body><h1>401 Unauthorized</h1></body></html>",
    Error("expected value", line: 1, column: 1),
)
[coins/monero/src/rpc/mod.rs:150] (raw, e) = (
    "<html><head><title>Unauthorized Access</title></head><body><h1>401 Unauthorized</h1></body></html>",
    Error("expected value", line: 1, column: 1),
)
thread 'tokio-runtime-worker' panicked at processor/src/networks/monero.rs:637:8:
called `Result::unwrap()` on an `Err` value: InvalidNode
[coins/monero/src/rpc/http.rs:60] e = reqwest::Error {
    kind: Request,
    url: Url {
        scheme: "http",
        cannot_be_a_base: false,
        username: "",
        password: None,
        host: Some(
            Ipv4(
                0.0.0.0,
            ),
        ),
        port: Some(
            33512,
        ),
        path: "/",
        query: None,
        fragment: None,
    },
    source: hyper::Error(
        IncompleteMessage,
    ),
}

I'm guessing this is stumbling into some mess predicated on Monero thinking we're a remote client, not a local one, and treating us with more hostility accordingly? I'll push my branch as-is, though it may be a distinct issue.

@kayabaNerve
Copy link
Member Author

kayabaNerve commented Oct 23, 2023 •
edited
Loading

Upon more review, the above looks to likely be an issue with our impl of authenticated RPCs.

I am concerned by the InvalidHeader though...

@kayabaNerve
Copy link
Member Author

kayabaNerve commented Oct 23, 2023 •
edited
Loading

I couldn't finish a run with authentication. Removing it, it seems to be fine. That's.... not good.

kayabaNerve added a commit that referenced this issue Oct 23, 2023
@kayabaNerve
Tweak how the Monero node is run for the processor tests
7c10873 
Disables the unused zmq RPC.

Removes authentication which seems to be unstable as hell when under load
(see #351).

No longer use Network::Isolated as it's not needed here (the Monero nodes run
with `--offline`).
@kayabaNerve
Copy link
Member Author

Presumably fixed by 3069138

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
critical This is critical monero An issue with the Monero library/integration
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@kayabaNerve @Boog900