Skip to content
This repository has been archived by the owner on Apr 15, 2024. It is now read-only.

feat: add testnode implementation #95

Merged
merged 9 commits into from
Feb 9, 2023
Merged

Conversation

rach-id
Copy link
Member

@rach-id rach-id commented Feb 2, 2023

Overview

Closes #55

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@rach-id rach-id added the testing label Feb 2, 2023
@rach-id rach-id requested a review from rahulghangas February 2, 2023 10:40
@rach-id rach-id self-assigned this Feb 2, 2023
@rach-id rach-id requested a review from evan-forbes as a code owner February 2, 2023 10:40
@codecov-commenter
Copy link

Codecov Report

Merging #95 (25eb4b9) into main (c1ac17e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #95   +/-   ##
=======================================
  Coverage   59.75%   59.75%           
=======================================
  Files           6        6           
  Lines         164      164           
=======================================
  Hits           98       98           
  Misses         53       53           
  Partials       13       13           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@evan-forbes
Copy link
Member

wow, this is really cool!

evan-forbes
evan-forbes previously approved these changes Feb 6, 2023
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

nice! took a second look, and this makes sense to me so far. LGTM, utACK 👍 👍

will defer to @rahulghangas for final approval

@rach-id
Copy link
Member Author

rach-id commented Feb 6, 2023

CI will be fixed by: #101

@rach-id
Copy link
Member Author

rach-id commented Feb 8, 2023

@rahulghangas Can you please check this so that I can open up new PRs?

Copy link
Contributor

@rahulghangas rahulghangas left a comment

Choose a reason for hiding this comment

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

LGTM, small comment (non blocking, can be merged without)

ChainID uint64
}

func NewEVMChain(key *ecdsa.PrivateKey, chainID uint64) *EVMChain {
Copy link
Contributor

Choose a reason for hiding this comment

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

To reduce future tech debt, can we add small doc comments for all exported functions and methods

Copy link
Member Author

Choose a reason for hiding this comment

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

I have docs for that in local. After this is merged, new PRs containing that will come. Thanks for flagging this

}
}

func (e *EVMChain) Close() {
Copy link
Contributor

Choose a reason for hiding this comment

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

As above

Copy link
Member Author

Choose a reason for hiding this comment

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

EVMChain *EVMChain
}

func NewTestNode(ctx context.Context, t *testing.T) *TestNode {
Copy link
Contributor

Choose a reason for hiding this comment

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

As above

Copy link
Member Author

Choose a reason for hiding this comment

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

}
}

func (tn TestNode) Close() {
Copy link
Contributor

Choose a reason for hiding this comment

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

As above

Copy link
Member Author

Choose a reason for hiding this comment

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

@rach-id rach-id merged commit acd5c2f into celestiaorg:main Feb 9, 2023
@rach-id rach-id deleted the add_testnode branch February 9, 2023 09:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a testnode for testing the QGB
4 participants