Addresses #173.

modifies the publish command to first query "our" node for its peers and then concurrently publish to each peer.

This is just a temporary feature until the p2p layer properly transmits code between nodes.

notes:

• This is kind of a draft, looking for feedback. but if you feel its ok to merge, go for it.
• I didn't bother with creating a flag/mode to publish only to "our" node (original behavior) because this is just a temporary thing, and I didn't want to put too much time/complexity into it. But if its important, let me know.
• seems to work as intended, but I haven't yet tested what happens when a node is slow or unavailable.
• I'm not a rust thread/tokio/async wizard, so I believe it is parallel+concurrent, but there could be some gotcha there, eg tokio decides to run all tasks on the same thread for whatever black box reason.

Edit by @steinerkelvin

TODO:

• [x] --hosts CLI parameter to override to which hosts to publish

addresses #247. (more to come)

This PR focuses on kindelia_core/src/util.rs.

util.rs

• u8s_to_u128s() now returns a Result instead of unwrap()
• get_time() and get_time_micro now returns UNIX_EPOCH (0u128) if system time is somehow before UNIX_EPOCH. rather than panic.

runtime/mod.rs

• now calls unwrap on result of u8s_to_u128s(). This is temporary until error handling is refactored in runtime.

note: regarding the change to get_time() and get_time_micro(). This change means that if a node's SystemTime is before Jan 1, 1970 00:00:00, then it will be interpreted as 1970-01-01 by a caller using the returned u128. I don't believe this should be a serious (security) issue because the p2p protocol should/must be verifying that blocks and messages between nodes are recent/current (or operating without time at all, eg using causal ordering, lamport timestamps, etc).

It would certainly be preferable to return a Result for these fn, but will require a lot of changes to node.rs and related code. Another option is to keep the panic() behavior in get_time() until the necessary node refactor can occur.

Trying to synchronize a local node with the testnet (quleuber:testnet branch) is apparently causing a message storm that clogs the node. It apparently receives the same block hundreds of times.

storm_log.txt

I understand that:

• the local node is asking for the same – or almost the same – block multiple times as it receives the tip from multiple nodes;
• receiving a new tip while it is still receiving the already asked blocks and building the intermediary waiting chain, thus triggering it to ask a block in the middle of the chunk it is already building;
• handling a single PutBlock message is expensive as it always hash the incoming block;
• handing a tip is particularly expensive as it has to traverse the waiting chain;

While understanding the actual cause(s) demands further investigation, I already have some optimizations in mind:

• sleep only the remaining time on each tick;
• preprocess all received messages on a given tick to issue only the necessary commands (add blocks, ask block, etc);
• memoize the association of (know tip –> root of waiting tree), effectively making the aforementioned traversal almost constant cost, only linear on newly added blocks;

cc @racs4

I was expecting to see kindelia node stop as counterpart to kindelia node start, but it doesn't exist.

Is it todo? If so, maybe I can take a stab at it.

Addresses #158.

obsoletes #218. (this PR applies to dev branch. #218 applies to master)

Implements 'completion' command to output shell completions for bash, powershell, fish, zsh, etc, as supported by clap_complete crate.

Introduces a dependency on clap_complete and update clap dep to 4.0.18.

$ kindelia completion --help
Generate auto-completion for a shell

Usage: kindelia completion <SHELL>

Arguments:
  <SHELL>  The shell to generate completion for [possible values: bash, elvish, fish, powershell, zsh]

Options:
  -h, --help  Print help information

Example of usage in bash.

$ kindelia completion bash > /tmp/completions.bash

$ source /tmp/completions.bash 

$ kindelia <tab>
--api        --config     -h           init         publish      sign         util         
-c           deserialize  help         node         run-remote   test         -V           
completion   get          --help       post         serialize    unserialize  --version 

$ kindelia publish <tab>
-e         --encoded  <FILE>     -h         --help     

I kept this impl as simple as possible. Future improvements could include:

• option to generate completions for all known shells and write each to a file.
• execute within build.rs to auto-generate completion files at compile time.

This panics on the second run because the first one is collecting/clearing the state of Plist.

ctr Nil 0
ctr Cons 2

fun PList 1 {
  !(PList $(Get)) = $(IO.load λx $(IO.done x))
} = $(Cons #111 $(Nil))

run {
  $(IO.call @PList $(Tuple1 $(Get)) λx
  $(IO.done x)
  )
}

run {
  $(IO.call @PList $(Tuple1 $(Get)) λx
  $(IO.done x)
  )
}

cc @racs4

Addresses #247 (more to come)

When combined with #265, all except 2 unwrap/expect are removed from node.rs. The remaining 2 expect occurrences are Mutex::lock().expect(), which I consider acceptable (necessary evil) for now.

This PR endeavors to keep the overall logic the same, except that:

• add_block() failure(s) results in logged error rather than panic. (logged by main init and tasks: do_handle_mined_block and receive_message)
• handle_request() failures due to inconsistent block data results in logged error rather than panic. (logged by main task receive_request)
• handle_message() failure due to "bad magic" (wrong network) results in logged error rather than silently failing. (logged by main task receive_message).
• get_time() is replaced by try_get_time() in a few places, and failure is logged in main() and main task loop is paused with explanatory log message (trying again each time it wakes up)

node.rs

• add 4 variants to NodeError
• return Result<_, NodeError> from add_block() and related Node methods.
• replace some expect calls with NodeError::Inconsistency
• return NodeError::BadMagic error from ::handle_message()
• log error in Node::main if receive_message task fails
• log error in Node::main if receive_request task fails
• log error in Node::main if handle_mined_block task fails
• log error and pause tasks in Node::main if try_get_time() fails
• replace get_time() with try_get_time() where possible

persistence.rs

• return Result from closure arg to BlockStorage::read_blocks()

util.rs

• change EpochError visibility: pub(crate) --> pub

Addresses issue #247. (part a: more to come later)

I started small by just fixing 4 unwrap() calls in kindelia_core/src/net.rs.

Unfortunately one of these involved a trait, which complicated things a bit and required small changes in other files.

In kindelia main.rs I could have just mapped the errors from net.rs into a string and submitted a smaller PR, but instead I decided it would be cleaner to handle them as anyhow::Result.

I ended up changing most fn in main.rs and config.rs to return anyhow::Result instead of Result<_, String>. This required some calls to map_err(|e| anyhow!(e) to deal with String errors from format!() or from lower level fn calls. However, as the lower level calls get fixed, then the map_err goes away and we will just have fn_call()?. very clean.

note: the map_err() is necessary because anyhow::Result accepts anyhow::Error which is a boxed dynamic error type that accepts any error type derived from std::error::Error. However String is not so derived and thus must be mapped (or preferably return anyhow::Result or a thiserror type instead).

So this is very much a preliminary / intermediate PR to introduce anyhow and show the direction.

I understand there is a big refactor happening in another branch. I am ok with rebasing onto it when merged. Or this could be merged first, whichever works.

kindelia_core:

• removes all unwrap() from net.rs
• defines error enums in net.rs: ProtoCommError and ParseAddressError
• return Result from affected fn in net.rs
• change IPV6 panic!() to unimplemented!()
• accept addr arg for Node::new() so it can avoid returning Result
• update test and bench as necessary

kindelia:

• add dep on anyhow crate
• return anyhow::Result from config methods
• return anyhow::Result from main methods, including main()
• return anyhow::Result from FileInput::read_to_string()

My noob-ish understanding of why SSTORE on ETH is expensive is that for proper decentralization, a node must be runnable by anyone who wants to. I‘m also understanding, that for the average node operator, at least today, the limiting factor is provisioning a machine with lots of disk space.

So two factors are limiting ETH to allow more tps:

• How much disk space is currently used
• How much additional disk space is necessary in the future (throughput).

In the Kindelua whitepaper, I then read:

Apps that involve huge amounts of state changes, such as games and exchanges, are too expensive on Ethereum, due to the high cost of the SSTORE opcode. Moreover, persisting state on Ethereum is complex and error-prone, due to the need of serializing structures into a map of 256-bit integers. Kindelia replaces the Merkle stores by reversible snapshots of HVM's heap, which makes the SSTORE opcode essentially free.

But my understanding had never been that persisting state in merkle trees has been the defining bottle neck. I can believe that this is a problem, but generally, I always thought that the limiting factor is configuring the ETH client such that a home staker can run it.

So in that case, if a hypothetical ETH implementation would integrate HVM‘s heap snapshots, they wouldn‘t reduce the gas cost of SSTORE, would they? Because the SSTORE has to be priced relatively to state growth. Or what am I missing?

Currently this contract

run {
  (/ #3 #0)
}

crashes kindelia, because the division operation is done as is (no checks are made):

Div => (*a_u).wrapping_div(*b_u) & NUM_MASK

and when b_u is 0, wrapping_div panics and crashes the whole program. This should be treated somehow, but what should be it's intended behaviour? Not reducing? Some kind of runtime error?

cc @steinerkelvin @VictorTaelin

related to #96

Possible solutions:

1. simple built-in typechecker
2. monotype contract interface (buffer etc)

FWIW, I think we should allow "sibling" functions to communicate with raw data somehow.

cc @racs4 @developedby @VictorTaelin

Addressing #247

This causes clippy to emit an error if unwrap, expect, or panic is used in any code invoked by kindelia/src/main.rs

It will therefore cause CI clippy check to fail if these are encountered.

Of course, the code may override with a #[allow(clippy::unwrap_used)]. So this is really just changing the default from allow to deny.

test cases are not affected. (panics are allowed)

Note that there are several ways to configure clippy lints. Once we've removed all the panics that we can, it will probably be best to apply these lints for the entire repo/workspace. But for now, this seems the simplest, and it applies to only non-test code in the kindelia crate which has already been scrubbed of unwrap, expect, panic.

addresses #247. (more to come)

This PR is intended to replace #265, or at least provide an alternative to it, for comparison by reviewer(s). I personally consider this version nicer/cleaner.

Here we impl a custom NodeBuilder that is more ergonomic for the caller than the version derived by derive_builder.

The builder pattern by its very nature involves some boilerplate with the setters, so there's no getting around that, but I've tried to keep it as simple as possible.

I considered putting NodeBuilder inside node.rs, but my reasoning is that node.rs is already quite large, and using a dedicated builder.rs we are able to remove a bit of code from it, rather than add to.

I included the previous commit that uses derive_builder to generate NodeBuilder, because I figure we might want to go back to it at some point, if derive_builder crate ever supports custom/transient properties as I requested.

I provided some basic doc-comments for the builder. Those more familiar with the codebase may wish to flesh the docs out more.

If the builder approach is deemed too heavy, then I guess the alternative is to use the pre-existing Node::new() except it must return a Result. I am willing to do that, but I do find this approach more flexible and pleasing. :-)

commit message:

Implements NodeBuilder which provides more ergonomic usage for callers than the derive_builder version, and also keeps Node clean without a lot of derive_builder annotations.

builder.rs: new file added

node.rs: * remove derive_builder annotations * remove NodeBuilder impl * remove ParseError variant from BlockBodyError * add comment that Node should be built with NodeBuilder

util.rs * make EpochError pub. It can bereturned by NodeBuilder::build()

lib.rs: * add builder mod

tests, bench, kindelia/src/main: * adjust usage of NodeBuilder

Presently we have a check for clippy errors, but badly formatted code is allowed.

Once some pending PRs have been reviewed+merged, I would be happy to make a PR that runs rustfmt over the entire repo, at which point we could enable a CI check and enforce rustfmt style going forward....

Node::add_block() is presently validating that timestamp of each block is greater than timestamp of parent block. This prevents any out-of-time-order blocks from occurring, which seems good, however it will lead to otherwise valid blocks from being included, and might lead to a lot of wasted cycles for some miners.

I have not looked deeply into this. There might be other ramifications, even possible attacks.

Bitcoin uses these rules:

1. A node will not accept a block whose timestamp is more than two hours in the future.
2. A node will not accept a block unless it has a timestamp greater than the median of the previous 11 blocks. In Bitcoin, we call this Median-Time-Past (MTP).

Apparently with Bitcoin blockchain, it is a common occurrence that miners have unsynced clocks and out-of-time-order blocks are created. quote:

it happens a ton! I was just looking at the last several weeks of block header data, there are a couple hundred blocks where timestamps are out of order - but only by a couple minutes, usually.

Clearly, if the kindelia rule were used, it would not "happen a ton", in fact it would never happen. Maybe this would only have the effect that all miners are more diligent about keeping their clocks synced using eg ntp. Or maybe it would mean some miners become unprofitable and thus choose not to mine anymore. If the latter, this is harmful to network security.

I am unsure of the right answer. I am just bringing this up for discussion.

It is probably worthwhile to review what other blockchain systems, in particular smart contract platforms have done.

Accomplishes three related goals:

1. remove unwraps() that were in Node::new()
2. enables/requires that a Node be instantiated infallibly
3. improves ergonomics for instantiating a Node

node.rs:

• add BlockBodyError::ParseError variant
• derive Builder for Node
• refactor Node::new() into:
  • NodeBuilder::genesis_code() and
  • NodeBuilder::build()

bench.rs, tests/network.rs, kindelia/src/main.rs:

• use NodeBuilder::build() instead of Node::new()