Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor SDK #2452

Merged
merged 27 commits into from
Dec 13, 2023
Merged

Refactor SDK #2452

merged 27 commits into from
Dec 13, 2023

Conversation

joshua-kim
Copy link
Contributor

@joshua-kim joshua-kim commented Dec 9, 2023

Why this should be merged

NewAppProtocol registers an application-defined handler and returns a client you can use to send messages against that protocol id. The problem is that there's kind of a circular dependency... if you have a handler that wants to use continue gossiping in its own AppGossip it's going to need a client, but the current api requires a handler to be instantiated before giving you a client to work with.

This PR aims to let you create Clients and Register Handlers in two separate APIs, so handlers that need to support sending messages inside of AppGossip can create clients and pass them into their handler instantiation.

How this works

Separates clients + handler logic

How this was tested

Existing UTs

@joshua-kim joshua-kim self-assigned this Dec 9, 2023
network/p2p/network.go Outdated Show resolved Hide resolved
@joshua-kim joshua-kim marked this pull request as ready for review December 9, 2023 09:29
Copy link
Contributor

@ceyonur ceyonur left a comment

Choose a reason for hiding this comment

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

just a nit

network/p2p/network.go Outdated Show resolved Hide resolved
@joshua-kim joshua-kim force-pushed the refactor-client branch 2 times, most recently from ab9c1ac to 59407bc Compare December 11, 2023 03:06
@joshua-kim joshua-kim added the cleanup Code quality improvement label Dec 11, 2023
@joshua-kim joshua-kim force-pushed the refactor-client branch 2 times, most recently from 3c26d85 to 02e7588 Compare December 11, 2023 05:01
joshua-kim and others added 5 commits December 12, 2023 13:04
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
@joshua-kim joshua-kim changed the base branch from dev to sender-refactor December 12, 2023 19:22
@joshua-kim joshua-kim changed the title Refactor client Refactor SDK Dec 12, 2023
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
network/p2p/network.go Outdated Show resolved Hide resolved
network/p2p/network.go Outdated Show resolved Hide resolved
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
@StephenButtolph StephenButtolph added this to the v1.10.18 milestone Dec 12, 2023
Base automatically changed from sender-refactor to dev December 12, 2023 23:05
Signed-off-by: Joshua Kim <[email protected]>
commit 82fbc97
Author: Stephen Buttolph <[email protected]>
Date:   Tue Dec 12 18:30:09 2023 -0500

    Add ACP signaling (#2476)

commit ac5a00e
Author: Joshua Kim <[email protected]>
Date:   Tue Dec 12 17:42:32 2023 -0500

    Refactor p2p unit tests (#2475)

    Signed-off-by: Joshua Kim <[email protected]>
    Co-authored-by: Dan Laine <[email protected]>

commit 0b2b109
Author: Dhruba Basu <[email protected]>
Date:   Tue Dec 12 16:48:28 2023 -0500

    `vms/platformvm`: Verify txs before building a block (#2359)

    Co-authored-by: Stephen Buttolph <[email protected]>

commit 4be744e
Author: Joshua Kim <[email protected]>
Date:   Tue Dec 12 15:08:48 2023 -0500

    P2P AppError handling (#2248)

    Signed-off-by: Joshua Kim <[email protected]>
    Co-authored-by: Stephen Buttolph <[email protected]>

commit 7963115
Author: Dhruba Basu <[email protected]>
Date:   Tue Dec 12 14:37:59 2023 -0500

    `vms/platformvm`: Add `TestBuildBlockForceAdvanceTime` test (#2472)

    Co-authored-by: Stephen Buttolph <[email protected]>

commit dc472ec
Author: Dhruba Basu <[email protected]>
Date:   Tue Dec 12 14:37:43 2023 -0500

    `vms/platformvm`: Permit usage of the `Transactions` field in `BanffProposalBlock` (#2451)

    Co-authored-by: Stephen Buttolph <[email protected]>

Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
joshua-kim and others added 6 commits December 12, 2023 19:16
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
@StephenButtolph StephenButtolph added this pull request to the merge queue Dec 13, 2023
Merged via the queue into dev with commit 832632a Dec 13, 2023
16 checks passed
@StephenButtolph StephenButtolph deleted the refactor-client branch December 13, 2023 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code quality improvement
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants