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

Move warp payload to avalanchego #923

Merged
merged 7 commits into from
Oct 12, 2023
Merged

Conversation

nytzuga
Copy link
Contributor

@nytzuga nytzuga commented Sep 29, 2023

Why this should be merged

This fixes ava-labs/avalanchego#2050 and ava-labs/avalanchego#2116

ava-labs/avalanchego#2116 and #920 should be merged before this PR

How this works

How this was tested

How is this documented

@nytzuga nytzuga self-assigned this Sep 29, 2023
@nytzuga nytzuga force-pushed the move-warp-payload-to-avalanchego branch from 742ed5e to 8bd462d Compare September 29, 2023 06:43
Copy link
Collaborator

@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.

could we change the target branch from master to warp-remove-destinationChainID-destinationAddress?

@nytzuga nytzuga changed the base branch from master to warp-remove-destinationChainID-destinationAddress September 29, 2023 15:54
@nytzuga
Copy link
Contributor Author

nytzuga commented Sep 29, 2023

@ceyonur done.

@nytzuga nytzuga marked this pull request as ready for review September 29, 2023 15:55
go.mod Outdated Show resolved Hide resolved
@nytzuga nytzuga force-pushed the move-warp-payload-to-avalanchego branch from db45c01 to d74711b Compare October 2, 2023 13:42
@ceyonur ceyonur added the DO NOT MERGE This PR is not meant to be merged in its current state label Oct 2, 2023
@ceyonur
Copy link
Collaborator

ceyonur commented Oct 2, 2023

I've added a DO NOT MERGE tag. We can remove once this is unblocked.

@nytzuga nytzuga force-pushed the move-warp-payload-to-avalanchego branch from d74711b to e2f1728 Compare October 2, 2023 13:49
@nytzuga nytzuga force-pushed the warp-remove-destinationChainID-destinationAddress branch from 7d8eaa9 to 82e7cff Compare October 2, 2023 19:28
@nytzuga nytzuga force-pushed the warp-remove-destinationChainID-destinationAddress branch 2 times, most recently from fd00831 to 8d8de76 Compare October 2, 2023 19:33
Base automatically changed from warp-remove-destinationChainID-destinationAddress to master October 3, 2023 19:54
@nytzuga nytzuga force-pushed the move-warp-payload-to-avalanchego branch 2 times, most recently from 549f9f5 to 7261bfa Compare October 3, 2023 20:17
@aaronbuchwald aaronbuchwald marked this pull request as draft October 4, 2023 21:44
@aaronbuchwald
Copy link
Collaborator

Converting to a draft until ava-labs/avalanchego#2116 is merged

}
return PackGetVerifiedWarpMessageOutput(GetVerifiedWarpMessageOutput{
Message: WarpMessage{
SourceChainID: common.Hash(warpMessage.SourceChainID),
OriginSenderAddress: addressedPayload.SourceAddress,
Payload: addressedPayload.Payload,
OriginSenderAddress: common.BytesToAddress(addressedCall.SourceAddress),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will make a follow up PR to switch from converting the bytes to an address to exposing the byte array directly to the caller and changing the Warp Solidity contract interface.

@ceyonur
Copy link
Collaborator

ceyonur commented Oct 11, 2023

We can undraft this after #952

@nytzuga nytzuga force-pushed the move-warp-payload-to-avalanchego branch from a705b1f to 05b25a0 Compare October 11, 2023 13:43
@nytzuga nytzuga force-pushed the move-warp-payload-to-avalanchego branch from 05b25a0 to b08dcef Compare October 11, 2023 14:05
@nytzuga nytzuga marked this pull request as ready for review October 12, 2023 05:28
@nytzuga nytzuga removed the DO NOT MERGE This PR is not meant to be merged in its current state label Oct 12, 2023
@nytzuga
Copy link
Contributor Author

nytzuga commented Oct 12, 2023

@ceyonur @aaronbuchwald This is ready for another round of reviews

@@ -21,6 +21,7 @@ import (
"github.com/ava-labs/avalanchego/utils/crypto/bls"
"github.com/ava-labs/avalanchego/utils/set"
avalancheWarp "github.com/ava-labs/avalanchego/vms/platformvm/warp"
avalancheWarpPayload "github.com/ava-labs/avalanchego/vms/platformvm/warp/payload"
Copy link
Contributor

Choose a reason for hiding this comment

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

q: why we need the alias? Can't we use payload as package name?

Copy link
Contributor

@abi87 abi87 left a comment

Choose a reason for hiding this comment

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

only a question about the package alias. Not sure we need to remark payload is an avalanche package in the package alias

The alias is not longer needed
@nytzuga
Copy link
Contributor Author

nytzuga commented Oct 12, 2023

@abi87 Great suggestion. It was needed before but it is not longer needed indeed and I dropped it at 2447fbe

@ceyonur ceyonur merged commit 326c660 into master Oct 12, 2023
8 checks passed
@ceyonur ceyonur deleted the move-warp-payload-to-avalanchego branch October 12, 2023 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

6 participants