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

chore: rename optimism to op-mainnet #422

Merged
merged 3 commits into from
Nov 20, 2024

Conversation

MuhtasimTanmoy
Copy link
Contributor

Closes #421

@ncitron
Copy link
Collaborator

ncitron commented Nov 2, 2024

Let's update the operationsolarstorm endpoint as well for consistency. I'll make sure to add an alias to that url.

@MuhtasimTanmoy
Copy link
Contributor Author

Let's update the operationsolarstorm endpoint as well for consistency. I'll make sure to add an alias to that url.

@ncitron Sure.

Btw, I ran cargo test-all after replacing .env with Alchemy API Key before pushing.

Output:

running 4 tests
test get_balance ... FAILED
test get_transaction_receipt ... FAILED
test call ... FAILED
test get_transaction_by_hash ... FAILED

failures:

---- get_balance stdout ----
thread 'get_balance' panicked at tests/rpc_equivalence.rs:22:59:
called `Result::unwrap()` on an `Err` value: NotPresent

---- get_transaction_receipt stdout ----
thread 'get_transaction_receipt' panicked at tests/rpc_equivalence.rs:22:59:
called `Result::unwrap()` on an `Err` value: NotPresent

---- call stdout ----
thread 'call' panicked at tests/rpc_equivalence.rs:22:59:
called `Result::unwrap()` on an `Err` value: NotPresent
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- get_transaction_by_hash stdout ----
thread 'get_transaction_by_hash' panicked at tests/rpc_equivalence.rs:22:59:
called `Result::unwrap()` on an `Err` value: NotPresent


failures:
    call
    get_balance
    get_transaction_by_hash
    get_transaction_receipt

test result: FAILED. 0 passed; 4 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.04s

Is this something I should be concerned about?

@ncitron
Copy link
Collaborator

ncitron commented Nov 2, 2024

Yeah thats fine. Those env vars get injected by our CI which is why it fails locally.

@ncitron
Copy link
Collaborator

ncitron commented Nov 5, 2024

Did you push your commit to update the URLs?

@MuhtasimTanmoy
Copy link
Contributor Author

MuhtasimTanmoy commented Nov 6, 2024

I'll make sure to add an alias to that URL.

What should be the new URLs?

@ncitron
Copy link
Collaborator

ncitron commented Nov 6, 2024

We should change all instances of optimism.operationsolarstorm.org to op-mainnet.operationsolarstorm.org

@MuhtasimTanmoy
Copy link
Contributor Author

And this?

file: opstack/src/server/net/block_handler.rs

blocks_v3_topic: IdentTopic::new(format!("/optimism/{}/2/blocks", chain_id)) 

@ncitron
Copy link
Collaborator

ncitron commented Nov 7, 2024

No this stays the same

@MuhtasimTanmoy
Copy link
Contributor Author

done

Copy link
Collaborator

@ncitron ncitron left a comment

Choose a reason for hiding this comment

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

LGTM!

@ncitron
Copy link
Collaborator

ncitron commented Nov 11, 2024

Before we merge can you run cargo fmt?

@MuhtasimTanmoy
Copy link
Contributor Author

Before we merge can you run cargo fmt?

Done. What about clippy errors?

@MuhtasimTanmoy
Copy link
Contributor Author

Any dependency?

@ncitron ncitron merged commit 362e9c9 into a16z:master Nov 20, 2024
3 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chore: rename optimism to op-mainnet
2 participants