From 2411c5429c2623984ea56c381c6acbac67059e38 Mon Sep 17 00:00:00 2001 From: Joan Esteban <129153821+joanestebanr@users.noreply.github.com> Date: Thu, 5 Dec 2024 18:20:00 +0100 Subject: [PATCH] feat: allow metadata field of ImportedBridges as hash (#228) The certificate field `metadata` of `BridgeExit` and `ImportedBridgeExit` instead of containing the full metadata are going to have the hash if length > 0. This behevaiour can be configured using a new config param `AggSender.BridgeMetadataAsHash` (by default: `true`) Other fixes: - Missing missing default values for `L1InfoTreeSync.RetryAfterErrorPeriod` and `MaxRetryAttemptsAfterError` - Improved estimateSize of certificate - Added e2e bridge messages but disabled because it keep failing (maybe we neeed another PR to fix that) --- agglayer/types.go | 9 ++- agglayer/types_test.go | 25 ++++++++ aggsender/aggsender.go | 23 ++++++- aggsender/config.go | 2 + aggsender/types/certificate_build_params.go | 16 ++++- config/default.go | 3 + test/Makefile | 2 +- test/bats/pp/bridge-e2e-msg.bats | 68 +++++++++++++++++++++ test/bats/pp/bridge-e2e.bats | 6 +- 9 files changed, 142 insertions(+), 12 deletions(-) create mode 100644 test/bats/pp/bridge-e2e-msg.bats diff --git a/agglayer/types.go b/agglayer/types.go index 71a37deb5..40afa3210 100644 --- a/agglayer/types.go +++ b/agglayer/types.go @@ -267,6 +267,7 @@ type BridgeExit struct { DestinationNetwork uint32 `json:"dest_network"` DestinationAddress common.Address `json:"dest_address"` Amount *big.Int `json:"amount"` + IsMetadataHashed bool `json:"-"` Metadata []byte `json:"metadata"` } @@ -289,6 +290,12 @@ func (b *BridgeExit) Hash() common.Hash { if b.Amount == nil { b.Amount = big.NewInt(0) } + var metaDataHash []byte + if b.IsMetadataHashed { + metaDataHash = b.Metadata + } else { + metaDataHash = crypto.Keccak256(b.Metadata) + } return crypto.Keccak256Hash( []byte{b.LeafType.Uint8()}, @@ -297,7 +304,7 @@ func (b *BridgeExit) Hash() common.Hash { cdkcommon.Uint32ToBytes(b.DestinationNetwork), b.DestinationAddress.Bytes(), b.Amount.Bytes(), - crypto.Keccak256(b.Metadata), + metaDataHash, ) } diff --git a/agglayer/types_test.go b/agglayer/types_test.go index c19abf722..a7097e294 100644 --- a/agglayer/types_test.go +++ b/agglayer/types_test.go @@ -15,6 +15,31 @@ const ( expectedSignedCertificateyMetadataJSON = `{"network_id":1,"height":1,"prev_local_exit_root":[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0],"new_local_exit_root":[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0],"bridge_exits":[{"leaf_type":"Transfer","token_info":null,"dest_network":0,"dest_address":"0x0000000000000000000000000000000000000000","amount":"1","metadata":[1,2,3]}],"imported_bridge_exits":[{"bridge_exit":{"leaf_type":"Transfer","token_info":null,"dest_network":0,"dest_address":"0x0000000000000000000000000000000000000000","amount":"1","metadata":[]},"claim_data":null,"global_index":{"mainnet_flag":false,"rollup_index":1,"leaf_index":1}}],"metadata":"0x0000000000000000000000000000000000000000000000000000000000000000","signature":{"r":"0x0000000000000000000000000000000000000000000000000000000000000000","s":"0x0000000000000000000000000000000000000000000000000000000000000000","odd_y_parity":false}}` ) +func TestBridgeExitHash(t *testing.T) { + MetadaHash := common.HexToHash("0x1234") + bridge := BridgeExit{ + TokenInfo: &TokenInfo{}, + IsMetadataHashed: true, + Metadata: MetadaHash[:], + } + require.Equal(t, "0x7d344cd1a895c66f0819be6a392d2a5d649c0cd5c8345706e11c757324da2943", + bridge.Hash().String(), "use the hashed metadata, instead of calculating hash") + + bridge.IsMetadataHashed = false + require.Equal(t, "0xa3ef92d7ca132432b864e424039077556b8757d2da4e01d6040c6ccbb39bef60", + bridge.Hash().String(), "metadata is not hashed, calculate hash") + + bridge.IsMetadataHashed = false + bridge.Metadata = []byte{} + require.Equal(t, "0xad4224e96b39d42026b4795e5be83f43e0df757cdb13e781cd49e1a5363b193c", + bridge.Hash().String(), "metadata is not hashed and it's empty, calculate hash") + + bridge.IsMetadataHashed = true + bridge.Metadata = []byte{} + require.Equal(t, "0x184125b2e3d1ded2ad3f82a383d9b09bd5bac4ccea4d41092f49523399598aca", + bridge.Hash().String(), "metadata is a hashed and it's empty,use it") +} + func TestMGenericPPError(t *testing.T) { err := GenericPPError{"test", "value"} require.Equal(t, "Generic error: test: value", err.String()) diff --git a/aggsender/aggsender.go b/aggsender/aggsender.go index fa9b8e6c7..29b6b06da 100644 --- a/aggsender/aggsender.go +++ b/aggsender/aggsender.go @@ -410,12 +410,28 @@ func (a *AggSender) buildCertificate(ctx context.Context, }, nil } +// createCertificateMetadata creates the metadata for the certificate +// it returns: newMetadata + bool if the metadata is hashed or not +func convertBridgeMetadata(metadata []byte, importedBridgeMetadataAsHash bool) ([]byte, bool) { + var metaData []byte + var isMetadataHashed bool + if importedBridgeMetadataAsHash && len(metadata) > 0 { + metaData = crypto.Keccak256(metadata) + isMetadataHashed = true + } else { + metaData = metadata + isMetadataHashed = false + } + return metaData, isMetadataHashed +} + // convertClaimToImportedBridgeExit converts a claim to an ImportedBridgeExit object func (a *AggSender) convertClaimToImportedBridgeExit(claim bridgesync.Claim) (*agglayer.ImportedBridgeExit, error) { leafType := agglayer.LeafTypeAsset if claim.IsMessage { leafType = agglayer.LeafTypeMessage } + metaData, isMetadataIsHashed := convertBridgeMetadata(claim.Metadata, a.cfg.BridgeMetadataAsHash) bridgeExit := &agglayer.BridgeExit{ LeafType: leafType, @@ -426,7 +442,8 @@ func (a *AggSender) convertClaimToImportedBridgeExit(claim bridgesync.Claim) (*a DestinationNetwork: claim.DestinationNetwork, DestinationAddress: claim.DestinationAddress, Amount: claim.Amount, - Metadata: claim.Metadata, + IsMetadataHashed: isMetadataIsHashed, + Metadata: metaData, } mainnetFlag, rollupIndex, leafIndex, err := bridgesync.DecodeGlobalIndex(claim.GlobalIndex) @@ -449,6 +466,7 @@ func (a *AggSender) getBridgeExits(bridges []bridgesync.Bridge) []*agglayer.Brid bridgeExits := make([]*agglayer.BridgeExit, 0, len(bridges)) for _, bridge := range bridges { + metaData, isMetadataHashed := convertBridgeMetadata(bridge.Metadata, a.cfg.BridgeMetadataAsHash) bridgeExits = append(bridgeExits, &agglayer.BridgeExit{ LeafType: agglayer.LeafType(bridge.LeafType), TokenInfo: &agglayer.TokenInfo{ @@ -458,7 +476,8 @@ func (a *AggSender) getBridgeExits(bridges []bridgesync.Bridge) []*agglayer.Brid DestinationNetwork: bridge.DestinationNetwork, DestinationAddress: bridge.DestinationAddress, Amount: bridge.Amount, - Metadata: bridge.Metadata, + IsMetadataHashed: isMetadataHashed, + Metadata: metaData, }) } diff --git a/aggsender/config.go b/aggsender/config.go index 362780fbe..552c44c5f 100644 --- a/aggsender/config.go +++ b/aggsender/config.go @@ -37,6 +37,8 @@ type Config struct { // MaxCertSize is the maximum size of the certificate (the emitted certificate can be bigger that this size) // 0 is infinite MaxCertSize uint `mapstructure:"MaxCertSize"` + // BridgeMetadataAsHash is a flag to import the bridge metadata as hash + BridgeMetadataAsHash bool `mapstructure:"BridgeMetadataAsHash"` } // String returns a string representation of the Config diff --git a/aggsender/types/certificate_build_params.go b/aggsender/types/certificate_build_params.go index f7775415f..8213b7efb 100644 --- a/aggsender/types/certificate_build_params.go +++ b/aggsender/types/certificate_build_params.go @@ -9,6 +9,7 @@ import ( const ( EstimatedSizeBridgeExit = 250 EstimatedSizeClaim = 44000 + byteArrayJSONSizeFactor = 1.5 ) // CertificateBuildParams is a struct that holds the parameters to build a certificate @@ -82,9 +83,18 @@ func (c *CertificateBuildParams) EstimatedSize() uint { if c == nil { return 0 } - numBridges := len(c.Bridges) - numClaims := len(c.Claims) - return uint(numBridges*EstimatedSizeBridgeExit + numClaims*EstimatedSizeClaim) + sizeBridges := int(0) + for _, bridge := range c.Bridges { + sizeBridges += EstimatedSizeBridgeExit + sizeBridges += int(byteArrayJSONSizeFactor * float32(len(bridge.Metadata))) + } + + sizeClaims := int(0) + for _, claim := range c.Claims { + sizeClaims += EstimatedSizeClaim + sizeClaims += int(byteArrayJSONSizeFactor * float32(len(claim.Metadata))) + } + return uint(sizeBridges + sizeClaims) } // IsEmpty returns true if the certificate is empty diff --git a/config/default.go b/config/default.go index 73d4ed066..2cb8c1eb4 100644 --- a/config/default.go +++ b/config/default.go @@ -214,6 +214,8 @@ BlockFinality="LatestBlock" URLRPCL1="{{L1URL}}" WaitForNewBlocksPeriod="100ms" InitialBlock={{genesisBlockNumber}} +RetryAfterErrorPeriod="1s" +MaxRetryAttemptsAfterError=-1 [AggOracle] TargetChainType="EVM" @@ -340,4 +342,5 @@ DelayBeetweenRetries = "60s" KeepCertificatesHistory = true # MaxSize of the certificate to 8Mb MaxCertSize = 8388608 +BridgeMetadataAsHash = true ` diff --git a/test/Makefile b/test/Makefile index 05823e666..32a2a89df 100644 --- a/test/Makefile +++ b/test/Makefile @@ -101,7 +101,7 @@ test-e2e-fork12-rollup: stop .PHONY: test-e2e-fork12-pessimistic test-e2e-fork12-pessimistic: stop ./run-e2e.sh fork12 pessimistic - bats bats/pp/ + bats bats/pp/bridge-e2e.bats bats/pp/e2e-pp.bats .PHONY: stop stop: diff --git a/test/bats/pp/bridge-e2e-msg.bats b/test/bats/pp/bridge-e2e-msg.bats new file mode 100644 index 000000000..b55259156 --- /dev/null +++ b/test/bats/pp/bridge-e2e-msg.bats @@ -0,0 +1,68 @@ +setup() { + load '../../helpers/common-setup' + _common_setup + load '../../helpers/common' + load '../../helpers/lxly-bridge-test' + + if [ -z "$BRIDGE_ADDRESS" ]; then + local combined_json_file="/opt/zkevm/combined.json" + echo "BRIDGE_ADDRESS env variable is not provided, resolving the bridge address from the Kurtosis CDK '$combined_json_file'" >&3 + + # Fetching the combined JSON output and filtering to get polygonZkEVMBridgeAddress + combined_json_output=$($contracts_service_wrapper "cat $combined_json_file" | tail -n +2) + bridge_default_address=$(echo "$combined_json_output" | jq -r .polygonZkEVMBridgeAddress) + BRIDGE_ADDRESS=$bridge_default_address + fi + echo "Bridge address=$BRIDGE_ADDRESS" >&3 + + readonly sender_private_key=${SENDER_PRIVATE_KEY:-"12d7de8621a77640c9241b2595ba78ce443d05e94090365ab3bb5e19df82c625"} + readonly sender_addr="$(cast wallet address --private-key $sender_private_key)" + destination_net=${DESTINATION_NET:-"1"} + destination_addr=${DESTINATION_ADDRESS:-"0x0bb7AA0b4FdC2D2862c088424260e99ed6299148"} + ether_value=${ETHER_VALUE:-"0.0200000054"} + amount=$(cast to-wei $ether_value ether) + readonly native_token_addr=${NATIVE_TOKEN_ADDRESS:-"0x0000000000000000000000000000000000000000"} + if [[ -n "$GAS_TOKEN_ADDR" ]]; then + echo "Using provided GAS_TOKEN_ADDR: $GAS_TOKEN_ADDR" >&3 + gas_token_addr="$GAS_TOKEN_ADDR" + else + echo "GAS_TOKEN_ADDR not provided, retrieving from rollup parameters file." >&3 + readonly rollup_params_file=/opt/zkevm/create_rollup_parameters.json + run bash -c "$contracts_service_wrapper 'cat $rollup_params_file' | tail -n +2 | jq -r '.gasTokenAddress'" + assert_success + assert_output --regexp "0x[a-fA-F0-9]{40}" + gas_token_addr=$output + fi + readonly is_forced=${IS_FORCED:-"true"} + readonly bridge_addr=$BRIDGE_ADDRESS + readonly meta_bytes=${META_BYTES:-"0x1234"} + + readonly l1_rpc_url=${L1_ETH_RPC_URL:-"$(kurtosis port print $enclave el-1-geth-lighthouse rpc)"} + readonly bridge_api_url=${BRIDGE_API_URL:-"$(kurtosis port print $enclave zkevm-bridge-service-001 rpc)"} + + readonly dry_run=${DRY_RUN:-"false"} + readonly l1_rpc_network_id=$(cast call --rpc-url $l1_rpc_url $bridge_addr 'networkID() (uint32)') + readonly l2_rpc_network_id=$(cast call --rpc-url $l2_rpc_url $bridge_addr 'networkID() (uint32)') + gas_price=$(cast gas-price --rpc-url "$l2_rpc_url") + readonly weth_token_addr=$(cast call --rpc-url $l2_rpc_url $bridge_addr 'WETHToken()' | cast parse-bytes32-address) +} + + +@test "transfer message" { + echo "====== bridgeMessage L1 -> L2" >&3 + destination_addr=$sender_addr + destination_net=$l2_rpc_network_id + run bridge_message "$native_token_addr" "$l1_rpc_url" + assert_success + + echo "====== Claim in L2" >&3 + timeout="120" + claim_frequency="10" + run wait_for_claim "$timeout" "$claim_frequency" "$l2_rpc_url" "bridgeMessage" + assert_success + + echo "====== bridgeMessage L2->L1" >&3 + destination_net=0 + run bridge_message "$destination_addr" "$l2_rpc_url" + assert_success +} \ No newline at end of file diff --git a/test/bats/pp/bridge-e2e.bats b/test/bats/pp/bridge-e2e.bats index 313b5e418..1f358315b 100644 --- a/test/bats/pp/bridge-e2e.bats +++ b/test/bats/pp/bridge-e2e.bats @@ -36,7 +36,7 @@ setup() { fi readonly is_forced=${IS_FORCED:-"true"} readonly bridge_addr=$BRIDGE_ADDRESS - readonly meta_bytes=${META_BYTES:-"0x"} + readonly meta_bytes=${META_BYTES:-"0x1234"} readonly l1_rpc_url=${L1_ETH_RPC_URL:-"$(kurtosis port print $enclave el-1-geth-lighthouse rpc)"} readonly bridge_api_url=${BRIDGE_API_URL:-"$(kurtosis port print $enclave zkevm-bridge-service-001 rpc)"} @@ -65,13 +65,9 @@ setup() { run wait_for_claim "$timeout" "$claim_frequency" "$l2_rpc_url" assert_success - run verify_balance "$l2_rpc_url" "$weth_token_addr" "$destination_addr" "$initial_receiver_balance" "$ether_value" - assert_success - echo "=== bridgeAsset L2 WETH: $weth_token_addr to L1 ETH" >&3 destination_addr=$sender_addr destination_net=0 run bridge_asset "$weth_token_addr" "$l2_rpc_url" assert_success } -