kindelia / Kindelia

An efficient, secure cryptocomputer
https://kindelia.org/
603 stars 39 forks source link

feat(core): load genesis file by network id #250

Open dan-da opened 1 year ago

dan-da commented 1 year ago

Addresses #243

Support for loading a different genesis block for each network.

edit: see this summary of latest changes

The genesis block is loaded from a file whose path is the pattern: ~/.kindelia/genesis/<network-id>.kdl

Changes:

core:

note: the error enums are based on the style/pattern recommended in this article, which is well worth a read. The pre-existing calling functions still call unwrap() on the result, but intent is to address that in #247.

cli:


I will note that this change might have security implications. In particular, when a genesis block is loaded from a static string in the executable then a checksum of the executable binary will also include the genesis block. By moving the block out into the filesystem we lose that guarantee. So both the binary and corresponding block file must be summed independently. Of course the benefit is that the system can cleanly handle an arbitrary number of networks. It could be that for mainnet, we use a hybrid approach, whereby mainnet and perhaps present testnet(s) genesis blocks are included in the binary, but others can be created by users at will.

dan-da commented 1 year ago

I think tests are failing in CI because the genesis file does not exist yet. They pass on my machine, where it does exist. Probably I need to update the kindelia init command to create this file. I thought about this a couple times and then forgot about it. I will look at it tmw.

steinerkelvin commented 1 year ago

update the kindelia init command to create this file

Totally. It should be done inside (or beside) init_config_file().

dan-da commented 1 year ago

Ok, so I just pushed a commit that does the kindelia init genesis install and also passes &[Statement] to core api instead of keying off network_id.

Getting the init functionality working was a bit tricky. I had to think about how these genesis files will be stored in the git repo and installed. I figured it makes sense that they would have same filename in the repo as in user's homedir. I put them in kindelia crate since all the loading is taking place there now.

Previously the genesis code was loaded into the executable with include_str!("genesis.kdl") macro, which works fine. But it requires a static string for the file path. Now I wanted the code to dynamically include the most recent version, eg "0xCAFE0006.kdl". That should be possible with a macro, but I am not experienced writing rust macros yet. So I found include_dir!() crate/macro that includes all files in a given directory. With this, I am able to sort and get the latest version for installation, which works.

However that gets me thinking: with this include_dir!() functionality including multiple genesis files directly in the source code, do we really need/want to load genesis files from disk at runtime? For purposes of testnets and mainnet, would it not be sufficient (and less complicated) to have a few directly in the binary? (and no need to install + load)

The tradeoff is that users would not be able to define their own networks without recompiling. I am unsure that users would typically need to define their own networks though...?

I hope I am being clear. ;-) See also the kindelia/genesis/README.md

dan-da commented 1 year ago

rebased to dev tip.

dan-da commented 1 year ago

I don't understand why the tests are failing in CI. The errors indicate that ~/.kindelia/genesis/0xCAFE0006.kdl is not found.

But it works fine for me, provided that I run kindelia init. Example:

$ cargo install --path .
...
$ mv ~/.kindelia/ ~/.kindelia.bak

$ kindelia init
Writing default configuration to '/home/danda/.kindelia/kindelia.toml'...

$ tree ~/.kindelia/
/home/danda/.kindelia/
├── genesis
│   └── 0xCAFE0006.kdl
└── kindelia.toml

1 directory, 2 files

$ cargo test
...
test result: ok. 37 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 8.44s

So either the CI is somehow not running kindelia init or there is something else different in the environment I do not understand.

steinerkelvin commented 1 year ago

I am unsure that users would typically need to define their own networks though

I think this is useful for development / experimentation. But it's indeed not strictly necessary.

With this, I am able to sort and get the latest version for installation, which works.

I was thinking of keeping include_str!("genesis.kdl"), hardcoding the "default" besides it (in this version 0xCAFE0006) that would be filled in the default.toml template to be written on the user config, and would be the filename for genesis.kdl when installed.

It's important to keep genesis.kdl static - somehow - because it's used in some tests. These unit tests should not depend on external files at all.

So either the CI is somehow not running kindelia init or there is something else different in the environment I do not understand.

It's indeed not run, this should not be necessary to run tests.

dan-da commented 1 year ago

It's important to keep genesis.kdl static - somehow - because it's used in some tests. These unit tests should not depend on external files at all.

It's indeed not run, this should not be necessary to run tests.

I thought I had accounted for this by creating genesis-tests.kdl which the tests include_str!().

What I missed is that some tests actually invoke the kindelia binary with a kindelia!() macro and then check its output. And since I had already run kindelia init locally, the test succeeded.

I think what I can do is change the test command to accept a file path as you suggest. Then the tests can pass that file to the binary.

Anyway, I will be sure and run the tests without the ~/.kindelia directory existing before pushing to this branch again.

dan-da commented 1 year ago

I went back to the drawing board a little on this, thinking about what is useful and makes sense for the end user.

The general approach is to make it easy for user to create a new network and genesis block for testing and local development, but also make it difficult to inadvertently screw up the genesis block for an "official" network.

Overview:

dan-da commented 1 year ago

rebased on latest dev.

dan-da commented 1 year ago

rebased on latest dev to resolve conflict.

dan-da commented 1 year ago

rebased on latest dev to resolve conflict.