Skip to content

Conversation

@pslaski
Copy link
Contributor

@pslaski pslaski commented Oct 2, 2020

Description

Ported Checkpointing JRC

Important Changes Introduced

  • new checkpointing RPC API
  • new endpoint - checkpointing_getLatestBlock
  • new endpoint checkpointing_pushCheckpoint

Testing

use insomnia endpoints

@pslaski pslaski requested review from ntallar and rtkaczyk October 2, 2020 11:25
@pslaski pslaski force-pushed the etcm-78-checkpointing-jrc branch 2 times, most recently from b107edd to b89ba0e Compare October 2, 2020 11:28
Copy link

@ntallar ntallar left a comment

Choose a reason for hiding this comment

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

Only minor comments and LGTM!

@pslaski pslaski force-pushed the etcm-78-checkpointing-jrc branch 2 times, most recently from 66a7921 to fe61eb4 Compare October 9, 2020 14:18
Copy link

@ntallar ntallar left a comment

Choose a reason for hiding this comment

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

Apart from a minor comment, LGTM!

response should haveResult(expectedResult)
}

it should "getLatestBlock in case of blockchain re-org" in new TestSetup {
Copy link

Choose a reason for hiding this comment

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

Isn't this more a test for the CheckpointingServiceSpec?

Copy link
Contributor

@rtkaczyk rtkaczyk left a comment

Choose a reason for hiding this comment

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

LGTM, except for Nico's comment about the test

@pslaski pslaski force-pushed the etcm-78-checkpointing-jrc branch from fe61eb4 to 6f36736 Compare October 12, 2020 11:06
@pslaski pslaski force-pushed the etcm-78-checkpointing-jrc branch from 6f36736 to 63dacbf Compare October 13, 2020 13:17
@pslaski pslaski force-pushed the etcm-78-checkpointing-jrc branch from 63dacbf to e6f4714 Compare October 13, 2020 14:07
Copy link

@ntallar ntallar left a comment

Choose a reason for hiding this comment

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

LGTM!

 .\\            //.
. \ \          / /.
.\  ,\     /` /,.-
 -.   \  /'/ /  .
 ` -   `-'  \  -
   '.       /.\`
      -    .-
      :`//.'
      .`.'
      .' BP 

@pslaski pslaski merged commit 8ae3eef into develop Oct 14, 2020
@pslaski pslaski deleted the etcm-78-checkpointing-jrc branch October 14, 2020 07:18
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.

4 participants