Skip to content
This repository was archived by the owner on Feb 8, 2026. It is now read-only.

🐛 fee config bug fix and target added to tx fee validations #156

Closed
garry-sharp wants to merge 1 commit into from
Closed

🐛 fee config bug fix and target added to tx fee validations #156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
43 changes: 30 additions & 13 deletions plugin/fees/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,13 @@ import (

// These are properties and parameters specific to the fee plugin config. They should be distinct from system/core config
type FeeConfig struct {
Type string `mapstructure:"type"`
Version string `mapstructure:"version"`
MaxFeeAmount uint64 `mapstructure:"max_fee_amount"` // Policies that are created/submitted which do not have this amount will be rejected.
UsdcAddress string `mapstructure:"usdc_address"` // The address of the USDC token on the Ethereum blockchain.
VerifierToken string `mapstructure:"verifier_token"` // The token to use for the verifier API.
chainId uint64 `mapstructure:"chain_id"` // The chain ID of the Ethereum blockchain.
ChainId *big.Int
EthProvider string `mapstructure:"eth_provider"` // The Ethereum provider to use for the fee plugin.
Type string `mapstructure:"type"`
Version string `mapstructure:"version"`
MaxFeeAmount uint64 `mapstructure:"max_fee_amount"` // Policies that are created/submitted which do not have this amount will be rejected.
UsdcAddress string `mapstructure:"usdc_address"` // The address of the USDC token on the Ethereum blockchain.
VerifierToken string `mapstructure:"verifier_token"` // The token to use for the verifier API.
ChainId *big.Int // The chain ID as a big.Int (initialized from ChainIdRaw).
EthProvider string `mapstructure:"eth_provider"` // The Ethereum provider to use for the fee plugin.
Jobs struct {
Load struct {
MaxConcurrentJobs uint64 `mapstructure:"max_concurrent_jobs"` //How many consecutive tasks can take place
Expand All @@ -34,6 +33,12 @@ type FeeConfig struct {
MaxConcurrentJobs uint64 `mapstructure:"max_concurrent_jobs"`
} `mapstructure:"post"`
}
DryRun bool `mapstructure:"dry_run"`
}

type FeeConfigFileWrapper struct {
FeeConfig `mapstructure:",squash"`
ChainIdRaw uint64 `mapstructure:"chain_id"`
}

type ConfigOption func(*FeeConfig) error
Expand All @@ -52,6 +57,7 @@ func withDefaults(c *FeeConfig) {
c.Jobs.Load.Cronexpr = "@every 2m"
c.Jobs.Transact.Cronexpr = "0 12 * * 5"
c.Jobs.Post.Cronexpr = "@every 5m"
c.DryRun = false
}

func WithMaxFeeAmount(maxFeeAmount uint64) ConfigOption {
Expand Down Expand Up @@ -100,9 +106,16 @@ func WithCronexpr(load, transact, post string) ConfigOption {
}
}

func WithFileConfig(basePath string) ConfigOption {
func WithDryRun(dryRun bool) ConfigOption {
return func(c *FeeConfig) error {
c.DryRun = dryRun
return nil
}
}

func WithFileConfig(basePath string) ConfigOption {

return func(c *FeeConfig) error {
v := viper.New()
v.SetConfigName("fee")

Expand All @@ -122,11 +135,15 @@ func WithFileConfig(basePath string) ConfigOption {
return fmt.Errorf("failed to read config: %w", err)
}

if err := v.Unmarshal(c); err != nil {
var wrappedConfig FeeConfigFileWrapper
if err := v.Unmarshal(&wrappedConfig); err != nil {
return fmt.Errorf("failed to unmarshal config: %w", err)
}

c.ChainId = big.NewInt(0).SetUint64(c.chainId)
// Copy all values from the wrapped config to the original config
*c = wrappedConfig.FeeConfig

c.ChainId = big.NewInt(0).SetUint64(wrappedConfig.ChainIdRaw)
return nil
Comment on lines +138 to 147

@coderabbitai coderabbitai Bot Aug 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Avoid clobbering defaults when loading from file

Overwriting the entire FeeConfig with the unmarshalled struct will zero out unspecified fields, losing defaults established by withDefaults. Prefer merging into the existing config and only overriding provided keys.

Apply this diff:

-		var wrappedConfig FeeConfigFileWrapper
-		if err := v.Unmarshal(&wrappedConfig); err != nil {
-			return fmt.Errorf("failed to unmarshal config: %w", err)
-		}
-
-		// Copy all values from the wrapped config to the original config
-		*c = wrappedConfig.FeeConfig
-
-		c.ChainId = big.NewInt(0).SetUint64(wrappedConfig.ChainIdRaw)
+		var wrappedConfig FeeConfigFileWrapper
+		if err := v.Unmarshal(&wrappedConfig); err != nil {
+			return fmt.Errorf("failed to unmarshal config: %w", err)
+		}
+		// Merge into existing defaults: only override fields provided in the file/env.
+		if err := v.Unmarshal(c); err != nil {
+			return fmt.Errorf("failed to unmarshal into FeeConfig: %w", err)
+		}
+		// Handle chain_id separately
+		if wrappedConfig.ChainIdRaw != 0 {
+			c.ChainId = big.NewInt(0).SetUint64(wrappedConfig.ChainIdRaw)
+		}

This change preserves defaults. If your config files include all fields, behavior remains unchanged; otherwise, this prevents unexpected zero-values.

🏁 Script executed:

#!/bin/bash
set -e

# Locate all config.go files
echo "Config files in repo:"
fd -t f -e go config.go

# Inspect the plugin/fees/config.go content around unmarshalling
echo -e "\n--- Context around v.Unmarshal in plugin/fees/config.go ---"
rg -n "v.Unmarshal" -C5 plugin/fees/config.go

# Find definitions of FeeConfigFileWrapper and FeeConfig
echo -e "\n--- Definition of FeeConfigFileWrapper ---"
rg -n "type FeeConfigFileWrapper" -C5

echo -e "\n--- Definition of FeeConfig ---"
rg -n "type FeeConfig" -C5

# Search for withDefaults usage
echo -e "\n--- Occurrences of withDefaults ---"
rg -n "withDefaults" -C5

Length of output: 4140

Merge file-loaded settings into existing defaults

Overwriting FeeConfig via *c = wrappedConfig.FeeConfig will zero out any fields not present in the file, dropping the defaults set by withDefaults. Instead, decode into the already-initialized struct so only provided keys override their defaults, and handle chain_id separately.

Locations to update:

  • plugin/fees/config.go: lines 138–147

Proposed diff:

-		var wrappedConfig FeeConfigFileWrapper
-		if err := v.Unmarshal(&wrappedConfig); err != nil {
-			return fmt.Errorf("failed to unmarshal config: %w", err)
-		}
-
-		// Copy all values from the wrapped config to the original config
-		*c = wrappedConfig.FeeConfig
-
-		c.ChainId = big.NewInt(0).SetUint64(wrappedConfig.ChainIdRaw)
-		return nil
+		var wrappedConfig FeeConfigFileWrapper
+		if err := v.Unmarshal(&wrappedConfig); err != nil {
+			return fmt.Errorf("failed to unmarshal config: %w", err)
+		}
+		// Merge into existing defaults: only override provided fields
+		if err := v.Unmarshal(c); err != nil {
+			return fmt.Errorf("failed to unmarshal into FeeConfig: %w", err)
+		}
+		// Override chain_id only if set in the file
+		if wrappedConfig.ChainIdRaw != 0 {
+			c.ChainId = big.NewInt(0).SetUint64(wrappedConfig.ChainIdRaw)
+		}
+		return nil
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var wrappedConfig FeeConfigFileWrapper
if err := v.Unmarshal(&wrappedConfig); err != nil {
return fmt.Errorf("failed to unmarshal config: %w", err)
}
c.ChainId = big.NewInt(0).SetUint64(c.chainId)
// Copy all values from the wrapped config to the original config
*c = wrappedConfig.FeeConfig
c.ChainId = big.NewInt(0).SetUint64(wrappedConfig.ChainIdRaw)
return nil
var wrappedConfig FeeConfigFileWrapper
if err := v.Unmarshal(&wrappedConfig); err != nil {
return fmt.Errorf("failed to unmarshal config: %w", err)
}
// Merge into existing defaults: only override provided fields
if err := v.Unmarshal(c); err != nil {
return fmt.Errorf("failed to unmarshal into FeeConfig: %w", err)
}
// Override chain_id only if set in the file
if wrappedConfig.ChainIdRaw != 0 {
c.ChainId = big.NewInt(0).SetUint64(wrappedConfig.ChainIdRaw)
}
return nil
🤖 Prompt for AI Agents 
In plugin/fees/config.go around lines 138–147, the code currently overwrites the
initialized FeeConfig with "*c = wrappedConfig.FeeConfig", which wipes defaults
set by withDefaults; instead, merge the file values into the existing struct so
only provided keys override defaults. Remove the full struct assignment and
copy/merge the fields from wrappedConfig.FeeConfig into *c (either
field-by-field or using a safe merge helper) so zero-value fields in the file do
not erase defaults, and still set c.ChainId explicitly via c.ChainId =
big.NewInt(0).SetUint64(wrappedConfig.ChainIdRaw).

}
}
Expand All @@ -149,8 +166,8 @@ func NewFeeConfig(fns ...ConfigOption) (*FeeConfig, error) {
return c, errors.New("verifier_token is required")
}

if c.ChainId == nil {
return c, errors.New("chain_id is required")
if c.ChainId == nil || c.ChainId.Uint64() == 0 {
return c, errors.New("chain_id is required and must not be 0")
}

if c.EthProvider == "" {
Expand Down
137 changes: 69 additions & 68 deletions plugin/fees/transaction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,88 +63,89 @@ func (fp *FeePlugin) proposeTransactions(ctx context.Context, policy vtypes.Plug

txs := []vtypes.PluginKeysignRequest{}
var magicConstantRecipientValue rtypes.MagicConstant = rtypes.MagicConstant_UNSPECIFIED
var token string

// This should only return one rule, but in case there are more/fewer rules, we'll loop through them all and error if it's the case.
for _, rule := range recipe.Rules {

// This section of code goes through the rules in the fee policy. It looks for the recipient of the fee collection policy and extracts it. If other data is found throws an error as they're unsupported rules.
var recipient string // The address specified in the fee policy.
switch rule.Resource {
case "ethereum.erc20.transfer":
for _, constraint := range rule.ParameterConstraints {
if constraint.ParameterName == "recipient" {
if constraint.Constraint.Type != rtypes.ConstraintType_CONSTRAINT_TYPE_MAGIC_CONSTANT {
return nil, fmt.Errorf("recipient constraint is not a magic constant")
}
iv, err := strconv.ParseInt(constraint.Constraint.GetFixedValue(), 10, 64)
if err != nil {
return nil, fmt.Errorf("failed to parse fixed value: %v", err)
}
magicConstantRecipientValue = rtypes.MagicConstant(iv)
}
if len(recipe.Rules) != 1 {
return nil, fmt.Errorf("expected 1 rule, got %d", len(recipe.Rules))
}
rule := recipe.Rules[0]

resourceName := "ethereum.erc20.transfer"
if rule.Resource != resourceName {
return nil, fmt.Errorf("rule resource expected to be %s", resourceName)
}

for _, constraint := range rule.ParameterConstraints {
if constraint.ParameterName == "recipient" {
if constraint.Constraint.Type != rtypes.ConstraintType_CONSTRAINT_TYPE_MAGIC_CONSTANT {
return nil, fmt.Errorf("recipient constraint is not a magic constant")
}
iv, err := strconv.ParseInt(constraint.Constraint.GetFixedValue(), 10, 64)
if err != nil {
return nil, fmt.Errorf("failed to parse fixed value: %v", err)
}
default:
return nil, fmt.Errorf("unsupported rule: %v", rule.Id)
magicConstantRecipientValue = rtypes.MagicConstant(iv)
}
}

if magicConstantRecipientValue != rtypes.MagicConstant_VULTISIG_TREASURY {
return nil, fmt.Errorf("recipient constraint is not a treasury magic constant")
}
if magicConstantRecipientValue != rtypes.MagicConstant_VULTISIG_TREASURY {
return nil, fmt.Errorf("recipient constraint is not a treasury magic constant")
}

treasuryResolver := resolver.NewDefaultTreasuryResolver()
recipient, _, err := treasuryResolver.Resolve(magicConstantRecipientValue, "ethereum", "usdc")
if err != nil {
return nil, fmt.Errorf("failed to resolve treasury address: %v", err)
}
treasuryResolver := resolver.NewDefaultTreasuryResolver()
recipient, _, err := treasuryResolver.Resolve(magicConstantRecipientValue, "ethereum", "usdc")
if err != nil {
return nil, fmt.Errorf("failed to resolve treasury address: %v", err)
}

if gcommon.HexToAddress(token) != gcommon.HexToAddress(usdc.Address) {
return nil, fmt.Errorf("token address does not match usdc address")
}
token := rule.Target.GetAddress()

amount := run.TotalAmount
if gcommon.HexToAddress(token) != gcommon.HexToAddress(usdc.Address) {
return nil, fmt.Errorf("token address does not match usdc address")
}

tx, err := fp.eth.MakeAnyTransfer(ctx,
gcommon.HexToAddress(ethAddress),
gcommon.HexToAddress(recipient),
gcommon.HexToAddress(usdc.Address),
big.NewInt(int64(amount)))
if err != nil {
return nil, fmt.Errorf("failed to generate unsigned transaction: %w", err)
}
amount := run.TotalAmount

txHex := hexutil.Encode(tx)
tx, err := fp.eth.MakeAnyTransfer(ctx,
gcommon.HexToAddress(ethAddress),
gcommon.HexToAddress(recipient),
gcommon.HexToAddress(usdc.Address),
big.NewInt(int64(amount)))
if err != nil {
return nil, fmt.Errorf("failed to generate unsigned transaction: %w", err)
}

txData, e := reth.DecodeUnsignedPayload(tx)
if e != nil {
return nil, fmt.Errorf("ethereum.DecodeUnsignedPayload: %w", e)
}
txHashToSign := etypes.LatestSignerForChainID(fp.config.ChainId).Hash(etypes.NewTx(txData))

msgHash := sha256.Sum256(txHashToSign.Bytes())

signRequest := vtypes.PluginKeysignRequest{
KeysignRequest: vtypes.KeysignRequest{
PublicKey: policy.PublicKey,
Messages: []vtypes.KeysignMessage{
{
Message: base64.StdEncoding.EncodeToString(txHashToSign.Bytes()),
RawMessage: txHex,
Chain: vcommon.Ethereum,
Hash: base64.StdEncoding.EncodeToString(msgHash[:]),
HashFunction: vtypes.HashFunction_SHA256,
},
txHex := hexutil.Encode(tx)

txData, e := reth.DecodeUnsignedPayload(tx)
if e != nil {
return nil, fmt.Errorf("ethereum.DecodeUnsignedPayload: %w", e)
}

txHashToSign := etypes.LatestSignerForChainID(fp.config.ChainId).Hash(etypes.NewTx(txData))
msgHash := sha256.Sum256(txHashToSign.Bytes())
signRequest := vtypes.PluginKeysignRequest{
KeysignRequest: vtypes.KeysignRequest{
PublicKey: policy.PublicKey,
Messages: []vtypes.KeysignMessage{
{
Message: base64.StdEncoding.EncodeToString(txHashToSign.Bytes()),
RawMessage: txHex,
Chain: vcommon.Ethereum,
Hash: base64.StdEncoding.EncodeToString(msgHash[:]),
HashFunction: vtypes.HashFunction_SHA256,
},
SessionID: "",
HexEncryptionKey: "",
PolicyID: policy.ID,
PluginID: policy.PluginID.String(),
},
Transaction: txHex,
}

txs = append(txs, signRequest)
SessionID: "",
HexEncryptionKey: "",
PolicyID: policy.ID,
PluginID: policy.PluginID.String(),
},
Transaction: txHex,
}

txs = append(txs, signRequest)

return txs, nil
}

Expand Down