Convert emergency hf bash script to ocaml #18053
Conversation
dkijania
changed the title
|
!ci-build-me |
1 similar comment
|
!ci-build-me |
dkijania
force-pushed
the
dkijania/move_canonical_block_conv
branch
from
f7259b1 to
ed45fbb
Compare
|
!ci-build-me |
|
!ci-build-me |
|
!ci-build-me |
|
!ci-build-me |
|
!ci-build-me |
dkijania
changed the title
|
|
||
| type genesis_block = { id : int; state_hash : string } | ||
|
|
||
| type block_info = { id : int; height : int64; protocol_version_id : int } |
There was a problem hiding this comment.
CREATE TABLE protocol_versions
( id serial PRIMARY KEY
, transaction int NOT NULL
, network int NOT NULL
, patch int NOT NULL
, UNIQUE (transaction,network,patch)
);protocol_version_id is just the primary key in this table. Since we've confirmed that in previous PR, we don't care about the stability of IDs, we should locate protocol version by the triple (transaction, network, patch) instead.
| FROM blocks | ||
| WHERE protocol_version_id = ? | ||
| AND global_slot_since_hard_fork = 0 | ||
| ORDER BY id ASC |
There was a problem hiding this comment.
Why would we every get more than 1 block?
There was a problem hiding this comment.
it is possible. if we do a emergency hard-fork without bumping protocol_version
There was a problem hiding this comment.
Oh wow.. That feels like a very corner case, I wasn't considering this in my previous work at all.
| Option.map info ~f:(fun (id, height, protocol_version_id) -> | ||
| { id; height; protocol_version_id } ) | ||
|
|
||
| let canonical_chain_members_query = |
There was a problem hiding this comment.
I'm not a fan of the fact we're relying on id being stable here.
| ~protocol_version = | ||
| let open Deferred.Result.Let_syntax in | ||
| let%map members = | ||
| Conn.collect_list canonical_chain_members_query |
There was a problem hiding this comment.
could reuse the input by refering $2 and $3 repetitively in query template.
| UPDATE blocks | ||
| SET chain_status = 'orphaned' | ||
| WHERE protocol_version_id = $1::int | ||
| AND (global_slot_since_genesis < $2::int OR $2::int IS NULL); |
There was a problem hiding this comment.
swap the param order so we're not relying on UNKNOWN vlaues
| WHERE protocol_version_id = $1::int | ||
| AND id = ANY ($2::int[]) | ||
| AND (global_slot_since_genesis < $3::int OR $3::int IS NULL); | ||
| |sql} |
There was a problem hiding this comment.
We can use something like
UPDATE blocks
SET chain_status = CASE
WHEN id = ANY($2::int[]) THEN 'canonical'
ELSE 'orphaned'
END
WHERE protocol_version_id = $1::int
AND ($3 IS NULL OR global_slot_since_genesis < $3::int);So to fuse these 2 mutations into 1.
Also we should rename them from query -> mutation.
There was a problem hiding this comment.
Yeah, good idea however we need to add another guardian to only change pending blocks. Looks ok, i will fix it
There was a problem hiding this comment.
Or maybe we don't need additional guardian ..
| open Caqti_request.Infix | ||
|
|
||
| (* Test database setup and teardown *) | ||
| module TestDb = struct |
There was a problem hiding this comment.
Add a warn here these are vulnerable to SQL injection and should never be exposed.
| in | ||
| let query = | ||
| ( Caqti_type.( | ||
| t3 (t4 int string (option int) string) (t4 int int int int) string) |
| module TestScenarios = struct | ||
| (* Test 1: Fork on canonical in the middle of new network *) | ||
| let test_fork_on_canonical_in_the_middle = | ||
| [ (1, "A", None, "0", 1, 0, 0, 2, "canonical") |
There was a problem hiding this comment.
Suggest: create record type instead of these tuples.
| Thread_safe.block_on_async_exn (fun () -> | ||
| test_convert_scenario conn_str scenario () ) | ||
|
|
||
| let conn_str_arg = |
There was a problem hiding this comment.
Please consider rewrite this with Core CLI parsing. We do have a couple of cmdliner in our codebase but they're very rare. And I think their readability is bad.
Converting old script which is supposed to convert pending blocks to canonical or orphaned based on emergency hardfork. Basic idea is that after HF we are left with some pending blocks after fork block. The first block of forker chain is canonical. Script goal was to convert all pending blocks between fork point and new chain blocks orphaned or canonical depending on chain membership (main chain -> canoncial, other -> orphaned)
Since we have toolbox for archive node hf related operations i took liberty of putting this implementation to ocaml in order to decrease maintenance cost of keeping such important logic in bash. I also migrated functional tests. No functionality should be changed