-
Notifications
You must be signed in to change notification settings - Fork 42
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
celestia #13
base: main
Are you sure you want to change the base?
celestia #13
Conversation
let run_cmd = vec![ | ||
"sh", | ||
"-c", | ||
"celestia light init --p2p.network=mocha > /home/celestia/keys.txt &&\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how will the computer recognise he celestia command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh ok, so we are pulling the image during init only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, we can keep on recreating the container and have same state as we are using a volume, I've in mind to prune the volume in case we detected that the setup was not proper
src/da/celestia.rs
Outdated
"celestia light init --p2p.network=mocha > /home/celestia/keys.txt &&\ | ||
celestia light auth admin --p2p.network=mocha > /home/celestia/auth.txt" | ||
]; | ||
run_celestia_light_node(run_cmd).await.unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unwrap, we can use eyre errors as an easy way for handling error, I used it in Ethereum.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, why do we need to run the light node here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't run it, I'll refactor it
src/da/celestia.rs
Outdated
]; | ||
run_celestia_light_node(run_cmd).await.unwrap(); | ||
// Waits for docker container to execute the commands and generate the keys | ||
thread::sleep(Duration::from_secs(5)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there no other way to handle this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if we don't wait for docker to complete the command execution that we called on line 52 the keys.txt
and auth.txt
files aren't properly formed and is empty when we try to build da_conf.json
. But yeah I think we can add some check to await if the container has exited or not because execution time will be system dependent and this is bad code, I was just hacking it to make it work, will update.
src/da/celestia.rs
Outdated
} | ||
} | ||
|
||
if address.eq("") || auth_token.eq("") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is_empty should work ig
src/da/celestia.rs
Outdated
// let start_cmd = vec![ | ||
// "sh", | ||
// "-c", | ||
// "celestia light init --p2p.network=mocha > /home/celestia/keys.txt &&celestia light auth admin \ | ||
// --p2p.network=mocha > /home/celestia/auth.txt &&celestia light start --core.ip=rpc-mocha.pops.one \ | ||
// --p2p.network=mocha", | ||
// ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's remove if not needed
src/utils/paths.rs
Outdated
fs::create_dir_all(&app_home)?; | ||
|
||
Ok(app_home) | ||
} | ||
|
||
pub fn get_celestia_home() -> Result<PathBuf, Error> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be moved to the celestia file, celestia is a specific DA layer, generic utils should have get_da_layer or generic names
No description provided.