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.

Sign up for GitHub

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

generate go bindings in one package per contract/library #270

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

generate go bindings in one package per contract/library #270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a9aa04b
generate go bindings in one package per contract/library
RnkSngh Dec 2, 2024
bc2b6dc
default to not running fork tests
RnkSngh Dec 2, 2024
4540d19
modify go bindings generation to fail if abi gen isn't found
RnkSngh Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/foundry.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ jobs:
env:
MODULE_ROOT_PATH: "./"
RPC_URL: ${{ secrets.FORK_RPC_URL }}
RUN_FORK_TEST: true
CHAIN_NAME: "fork-test-ci"
DUMMY_DEPLOYER_PRIVATE_KEY: ${{ secrets.DUMMY_DEPLOYER_PRIVATE_KEY }}
DAPP_PRIVATE_KEY_1: ${{ secrets.DAPP_PRIVATE_KEY_1 }}
Expand Down
15 changes: 10 additions & 5 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ CONTRACT_JSON_FILES = $(filter-out $(CONTRACT_ABI_FILES),$(CONTRACT_BOTH_FILES))
.PHONY: build-contracts bindings-gen-go bindings-gen-ts

build-contracts:
forge --version || exit 1; \
echo "Building contracts"; \
rm -frd ./out; \
forge install; \
Expand All @@ -54,7 +55,9 @@ build-contracts:
# as they are not publicly exposed, but rather used within the contract itself.
#
# ABIGen issue ref: https://github.com/ethereum/solidity/issues/9278
bindings-gen-go: build-contracts
bindings-gen-go: build-contracts
go version || exit 1; \
abigen --version || exit 1; \
echo "Generating Go vIBC bindings..."; \
rm -rfd ./bindings/go/* ; \
for abi_file in $(CONTRACT_ABI_FILES); do \
Expand All @@ -63,14 +66,16 @@ bindings-gen-go: build-contracts
continue; \
fi; \
type=$$(basename $$abi_file .abi.json); \
pkg=$$(basename $$abi_base .sol | tr "[:upper:]" "[:lower:]"); \
pkg=$$(basename $$type .sol | tr "[:upper:]" "[:lower:]"); \
RnkSngh marked this conversation as resolved.
Show resolved Hide resolved
mkdir -p ./bindings/go/$$pkg; \
abigen --abi $$abi_file --pkg $$pkg --type $$type --out ./bindings/go/$$pkg/$$type.go; \
abigen --abi $$abi_file --pkg $$pkg --type $$type --out ./bindings/go/$$pkg/$$type.go || exit 1; \
done; \
echo "Done."
echo Running sanity check on go bindings ; \
go build ./... || exit 1; \
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: you need the || exit 1. If go build fails, the make target will fail too.

Copy link
Contributor

Choose a reason for hiding this comment

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

nit2: maybe add a message saying "running check sanity on bindings" or similar?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(assuming you meant I don't need the exit 1 )

I think we actually do need this exit 1 to cause the root workflow to fail ; not sure if this is default behavior or maybe it's an os thing but the make command doesn't fail of the go build fails without the exit 1 - I tried this myself through replacing the go build ./... with go build ./. to cause it to fail due to not finding any files to build. Running the failing command without the exit command still causes the overall make command to pass.

Similar to the behavior we're seeing with abigen

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

re second nit though I added messaging as you requested, thanks for the suggestion

Copy link
Contributor

@nicopernas nicopernas Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, I did mean "don't need" LOL

Make should propagate the errors thrown by shell commands executed by targets. Note that each line within a target is handled by a subshell. In your case you only have one command (single subshell) since you are concatenating commands like so cmd0; cmd1; cmd2; ... in which case you do have to exit

Here's a clear example

[nicolas@macbook foo]# cat Makefile
fail:
        @echo hello
        @go build ./.
        @echo bye
shell:
        @echo hello; \
        go build ./.; \
        echo bye;
exit:
        @echo hello; \
        go build ./. || exit 1; \
        echo bye;

[nicolas@macbook foo]# make fail ; echo ">> $?"
hello
package .: no Go files in /private/tmp/foo
make: *** [fail] Error 1
>> 2

[nicolas@macbook foo]# make shell ; echo ">> $?"
hello
package .: no Go files in /private/tmp/foo
bye
>> 0

[nicolas@macbook foo]# make exit ; echo ">> $?"
hello
package .: no Go files in /private/tmp/foo
make: *** [exit] Error 1
>> 2

Things like for loops need to be within a single subshell (so you need the ; \) but other things could be in separate ones. Not a big deal though - it's ok as is.

echo "Successfully generated go bindings."

bindings-gen-ts: build-contracts
echo "Generating TypeScript bindings..."; \
rm -rfd ./src/evm/contracts/*; \
typechain --target ethers-v6 --out-dir ./src/evm/contracts $(CONTRACT_JSON_FILES); \
echo "Done."
echo "Successfully generated ts bindings."
2 changes: 1 addition & 1 deletion ...gs/go/iproofverifier/IAppStateVerifier.go → ...go/iappstateverifier/IAppStateVerifier.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion ...ings/go/ibcdispatcher/IbcEventsEmitter.go → ...s/go/ibceventsemitter/IbcEventsEmitter.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bindings/go/ibcdispatcher/IbcPacketSender.go → ...ngs/go/ibcpacketsender/IbcPacketSender.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bindings/go/ilightclient/IClientUpdates.go → bindings/go/iclientupdates/IClientUpdates.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bindings/go/mars/PanickingMars.go → bindings/go/panickingmars/PanickingMars.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bindings/go/channel/ProtoChannel.go → bindings/go/protochannel/ProtoChannel.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bindings/go/channel/ProtoCounterparty.go → ...go/protocounterparty/ProtoCounterparty.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bindings/go/mars/RevertingBytesMars.go → .../revertingbytesmars/RevertingBytesMars.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bindings/go/mars/RevertingEmptyMars.go → .../revertingemptymars/RevertingEmptyMars.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion ...o/mars/RevertingStringCloseChannelMars.go → ...elmars/RevertingStringCloseChannelMars.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bindings/go/mars/RevertingStringMars.go → ...evertingstringmars/RevertingStringMars.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

119 changes: 65 additions & 54 deletions test/Fork/Dispatcher.deploy.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,65 +47,76 @@ contract DispatcherDeployTest is ChannelHandShakeUpgradeUtil, UpgradeTestUtils {
string portPrefix1 = "polyibc.eth1.";
string portId1 = "polyibc.eth1.71C95911E9a5D330f4D621842EC243EE1343292e";
string portId2 = "polyibc.eth2.71C95911E9a5D330f4D621842EC243EE1343292e";
bool skipFork = !vm.envOr("RUN_FORK_TEST", false); // we default to not running tests

function setUp() public override {
ChainAddresses memory addresses = ChainAddresses(
IDispatcher(vm.envAddress("DispatcherProxy")),
IUniversalChannelHandler(vm.envAddress("UCProxy")),
ILightClient(vm.envAddress("OptimisticLightClient")),
vm.envAddress("OwnerAddress")
);

opLightClient = addresses.optimisticLightClient; // Need to set this so that when we call load_proof, it loads
// the proof to the right address

mars = Mars(payable(targetMarsAddress));

dispatcherProxy = addresses.dispatcherProxy;
vm.prank(addresses.owner);

// For now, we need to change the portPrefix to that of the one which was used to generate the proof. We also
// have to set that for the connectionHop to light client mapping.
// Ideally, we can move to eventually automatically generating the proof by querying on chain.
dispatcherProxy.setPortPrefix(portPrefix1);
// Use legacy mars implementation to test upgrade compatibility
deployCodeTo("contracts/examples/Mars.sol:Mars", abi.encode(address(dispatcherProxy)), targetMarsAddress);

// We have to set the connection hops to hard-coded values since these will be checked in the proof
connectionHops0 = ["connection-0", "connection-3"];
connectionHops1 = ["connection-2", "connection-1"];

vm.startPrank(addresses.owner); // Only sender should have permission
dispatcherProxy.setClientForConnection("connection-0", opLightClient);

dispatcherProxy.setClientForConnection("connection-2", opLightClient);
vm.stopPrank();

_local = LocalEnd(IbcChannelReceiver(targetMarsAddress), portId1, "channel-0", connectionHops0, "1.0", "1.0");
_remote = ChannelEnd(portId2, "channel-1", "1.0");

// Do handshake as if Mars was the sending localparty
doSrcProofChannelHandshake(_local, _remote);

// Do handshake as if Mars was the receiving counterparty
doDestProofChannelHandshake(
_remote,
ChannelEnd(_local.portId, _local.channelId, _local.versionCall),
connectionHops1,
_local.versionExpected,
mars
);

// State should be initialized correctly & not impacted after any upgrades; used to check for malformatted state
uint64 nextSequenceRecvValue =
uint64(uint256(vm.load(address(dispatcherProxy), findNextSequenceRecv(address(mars), _local.channelId))));
uint64 nextSequenceAckValue =
uint64(uint256(vm.load(address(dispatcherProxy), findNextSequenceAck(address(mars), _local.channelId))));
assertEq(1, nextSequenceRecvValue);
assertEq(1, nextSequenceAckValue);
if (!skipFork) {
ChainAddresses memory addresses = ChainAddresses(
IDispatcher(vm.envAddress("DispatcherProxy")),
IUniversalChannelHandler(vm.envAddress("UCProxy")),
ILightClient(vm.envAddress("OptimisticLightClient")),
vm.envAddress("OwnerAddress")
);

opLightClient = addresses.optimisticLightClient; // Need to set this so that when we call load_proof, it
// loads
// the proof to the right address

mars = Mars(payable(targetMarsAddress));

dispatcherProxy = addresses.dispatcherProxy;
vm.prank(addresses.owner);

// For now, we need to change the portPrefix to that of the one which was used to generate the proof. We
// also
// have to set that for the connectionHop to light client mapping.
// Ideally, we can move to eventually automatically generating the proof by querying on chain.
dispatcherProxy.setPortPrefix(portPrefix1);
// Use legacy mars implementation to test upgrade compatibility
deployCodeTo("contracts/examples/Mars.sol:Mars", abi.encode(address(dispatcherProxy)), targetMarsAddress);

// We have to set the connection hops to hard-coded values since these will be checked in the proof
connectionHops0 = ["connection-0", "connection-3"];
connectionHops1 = ["connection-2", "connection-1"];

vm.startPrank(addresses.owner); // Only sender should have permission
dispatcherProxy.setClientForConnection("connection-0", opLightClient);

dispatcherProxy.setClientForConnection("connection-2", opLightClient);
vm.stopPrank();

_local =
LocalEnd(IbcChannelReceiver(targetMarsAddress), portId1, "channel-0", connectionHops0, "1.0", "1.0");
_remote = ChannelEnd(portId2, "channel-1", "1.0");

// Do handshake as if Mars was the sending localparty
doSrcProofChannelHandshake(_local, _remote);

// Do handshake as if Mars was the receiving counterparty
doDestProofChannelHandshake(
_remote,
ChannelEnd(_local.portId, _local.channelId, _local.versionCall),
connectionHops1,
_local.versionExpected,
mars
);

// State should be initialized correctly & not impacted after any upgrades; used to check for malformatted
// state
uint64 nextSequenceRecvValue = uint64(
uint256(vm.load(address(dispatcherProxy), findNextSequenceRecv(address(mars), _local.channelId)))
);
uint64 nextSequenceAckValue =
uint64(uint256(vm.load(address(dispatcherProxy), findNextSequenceAck(address(mars), _local.channelId))));
assertEq(1, nextSequenceRecvValue);
assertEq(1, nextSequenceAckValue);
} else {
console2.log("skipping fork test since RUN_FORK_TEST was not set to true");
}
}

function test_Deployment_SentPacketState_Conserved() public {
vm.skip(skipFork); // Skip if we don't opt-in to running fork tests
// Send a few packets as if mars was sending party
payload = "packet-1"; // Overwrite these values for inheriting class
packet = abi.encodePacked(payload);
Expand Down
Loading