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

fix(contracts): remove ExceededMaxRespondFee error #1407

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 10 additions & 9 deletions aggregator/pkg/aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,15 @@ type Aggregator struct {
func NewAggregator(aggregatorConfig config.AggregatorConfig) (*Aggregator, error) {
newBatchChan := make(chan *servicemanager.ContractAlignedLayerServiceManagerNewBatchV3)

logger := aggregatorConfig.BaseConfig.Logger

// Metrics
reg := prometheus.NewRegistry()
aggregatorMetrics := metrics.NewMetrics(aggregatorConfig.Aggregator.MetricsIpPortAddress, reg, logger)

// Telemetry
aggregatorTelemetry := NewTelemetry(aggregatorConfig.Aggregator.TelemetryIpPortAddress, logger)

avsReader, err := chainio.NewAvsReaderFromConfig(aggregatorConfig.BaseConfig, aggregatorConfig.EcdsaConfig)
if err != nil {
return nil, err
Expand All @@ -103,7 +112,7 @@ func NewAggregator(aggregatorConfig config.AggregatorConfig) (*Aggregator, error
return nil, err
}

avsWriter, err := chainio.NewAvsWriterFromConfig(aggregatorConfig.BaseConfig, aggregatorConfig.EcdsaConfig)
avsWriter, err := chainio.NewAvsWriterFromConfig(aggregatorConfig.BaseConfig, aggregatorConfig.EcdsaConfig, aggregatorMetrics)
if err != nil {
return nil, err
}
Expand All @@ -124,7 +133,6 @@ func NewAggregator(aggregatorConfig config.AggregatorConfig) (*Aggregator, error

aggregatorPrivateKey := aggregatorConfig.EcdsaConfig.PrivateKey

logger := aggregatorConfig.BaseConfig.Logger
clients, err := sdkclients.BuildAll(chainioConfig, aggregatorPrivateKey, logger)
if err != nil {
logger.Errorf("Cannot create sdk clients", "err", err)
Expand All @@ -150,13 +158,6 @@ func NewAggregator(aggregatorConfig config.AggregatorConfig) (*Aggregator, error
avsRegistryService := avsregistry.NewAvsRegistryServiceChainCaller(avsReader.ChainReader, operatorPubkeysService, logger)
blsAggregationService := blsagg.NewBlsAggregatorService(avsRegistryService, hashFunction, logger)

// Metrics
reg := prometheus.NewRegistry()
aggregatorMetrics := metrics.NewMetrics(aggregatorConfig.Aggregator.MetricsIpPortAddress, reg, logger)

// Telemetry
aggregatorTelemetry := NewTelemetry(aggregatorConfig.Aggregator.TelemetryIpPortAddress, logger)

nextBatchIndex := uint32(0)

aggregator := Aggregator{
Expand Down

Large diffs are not rendered by default.

16 changes: 7 additions & 9 deletions contracts/src/core/AlignedLayerServiceManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -214,20 +214,18 @@ contract AlignedLayerServiceManager is
// 70k was measured by trial and error until the aggregator got paid a bit over what it needed
uint256 txCost = (initialGasLeft - gasleft() + 70_000) * tx.gasprice;

if (txCost > currentBatch.respondToTaskFeeLimit) {
revert ExceededMaxRespondFee(
currentBatch.respondToTaskFeeLimit,
txCost
);
}
// limit amount to spend is respondToTaskFeeLimit
uint256 transferAmount = txCost < currentBatch.respondToTaskFeeLimit ?
txCost : currentBatch.respondToTaskFeeLimit;

batchersBalances[senderAddress] -= transferAmount;

// Subtract the txCost from the batcher's balance
batchersBalances[senderAddress] -= txCost;
emit BatcherBalanceUpdated(
senderAddress,
batchersBalances[senderAddress]
);
payable(alignedAggregator).transfer(txCost);

payable(alignedAggregator).transfer(transferAmount);
}

function isVerifierDisabled(
Expand Down
1 change: 0 additions & 1 deletion contracts/src/core/IAlignedLayerServiceManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ interface IAlignedLayerServiceManager {
error InvalidQuorumThreshold(uint256 signedStake, uint256 requiredStake); // a61eb88a
error SenderIsNotAggregator(address sender, address alignedAggregator); // 2cbe4195
error InvalidDepositAmount(uint256 amount); // 412ed242
error ExceededMaxRespondFee(uint256 respondToTaskFeeLimit, uint256 txCost); // 86fc507e
error InvalidAddress(string param); // 161eb542

function createNewTask(
Expand Down
63 changes: 40 additions & 23 deletions core/chainio/avs_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
retry "github.com/yetanotherco/aligned_layer/core"
"github.com/yetanotherco/aligned_layer/core/config"
"github.com/yetanotherco/aligned_layer/core/utils"
"github.com/yetanotherco/aligned_layer/metrics"
)

type AvsWriter struct {
Expand All @@ -27,9 +28,10 @@ type AvsWriter struct {
Signer signer.Signer
Client eth.InstrumentedClient
ClientFallback eth.InstrumentedClient
metrics *metrics.Metrics
}

func NewAvsWriterFromConfig(baseConfig *config.BaseConfig, ecdsaConfig *config.EcdsaConfig) (*AvsWriter, error) {
func NewAvsWriterFromConfig(baseConfig *config.BaseConfig, ecdsaConfig *config.EcdsaConfig, metrics *metrics.Metrics) (*AvsWriter, error) {

buildAllConfig := clients.BuildAllConfig{
EthHttpUrl: baseConfig.EthRpcUrl,
Expand Down Expand Up @@ -69,6 +71,7 @@ func NewAvsWriterFromConfig(baseConfig *config.BaseConfig, ecdsaConfig *config.E
Signer: privateKeySigner,
Client: baseConfig.EthRpcClient,
ClientFallback: baseConfig.EthRpcClientFallback,
metrics: metrics,
}, nil
}

Expand All @@ -83,11 +86,6 @@ func (w *AvsWriter) SendAggregatedResponse(batchIdentifierHash [32]byte, batchMe
return nil, err
}

err = w.checkRespondToTaskFeeLimit(tx, txOpts, batchIdentifierHash, senderAddress)
if err != nil {
return nil, err
}

// Set the nonce, as we might have to replace the transaction with a higher gas price
txNonce := big.NewInt(int64(tx.Nonce()))
txOpts.Nonce = txNonce
Expand All @@ -114,7 +112,9 @@ func (w *AvsWriter) SendAggregatedResponse(batchIdentifierHash [32]byte, batchMe
onGasPriceBumped(txOpts.GasPrice)
}

err = w.checkRespondToTaskFeeLimit(tx, txOpts, batchIdentifierHash, senderAddress)
// We compare both Aggregator funds and Batcher balance in Aligned against respondToTaskFeeLimit
// Both are required to have some balance, more details inside the function
err = w.checkAggAndBatcherHaveEnoughBalance(tx, txOpts, batchIdentifierHash, senderAddress)
if err != nil {
return nil, retry.PermanentError{Inner: err}
}
Expand All @@ -128,6 +128,7 @@ func (w *AvsWriter) SendAggregatedResponse(batchIdentifierHash [32]byte, batchMe

receipt, err := utils.WaitForTransactionReceiptRetryable(w.Client, w.ClientFallback, tx.Hash(), timeToWaitBeforeBump)
if receipt != nil {
w.checkIfAggregatorHadToPaidForBatcher(tx, batchIdentifierHash)
return receipt, nil
}

Expand All @@ -145,29 +146,45 @@ func (w *AvsWriter) SendAggregatedResponse(batchIdentifierHash [32]byte, batchMe
return retry.RetryWithData(respondToTaskV2Func, retry.MinDelay, retry.RetryFactor, 0, retry.MaxInterval, 0)
}

func (w *AvsWriter) checkRespondToTaskFeeLimit(tx *types.Transaction, txOpts bind.TransactOpts, batchIdentifierHash [32]byte, senderAddress [20]byte) error {
aggregatorAddress := txOpts.From
simulatedCost := new(big.Int).Mul(new(big.Int).SetUint64(tx.Gas()), tx.GasPrice())
w.logger.Info("Simulated cost", "cost", simulatedCost)

// Get RespondToTaskFeeLimit
// Calculates the transaction cost from the receipt and compares it with the batcher respondToTaskFeeLimit
// if the tx cost was higher, then it means the aggregator has paid the difference for the batcher (txCost - respondToTaskFeeLimit) and so metrics are updated accordingly.
// otherwise nothing is done.
func (w *AvsWriter) checkIfAggregatorHadToPaidForBatcher(tx *types.Transaction, batchIdentifierHash [32]byte) {
batchState, err := w.BatchesStateRetryable(&bind.CallOpts{}, batchIdentifierHash)
if err != nil {
// Fallback also failed
// Proceed to check values against simulated costs
w.logger.Error("Failed to get batch state", "error", err)
w.logger.Info("Proceeding with simulated cost checks")
return w.compareBalances(simulatedCost, aggregatorAddress, senderAddress)
return
}
// At this point, batchState was successfully retrieved
// Proceed to check values against RespondToTaskFeeLimit
respondToTaskFeeLimit := batchState.RespondToTaskFeeLimit
w.logger.Info("Batch RespondToTaskFeeLimit", "RespondToTaskFeeLimit", respondToTaskFeeLimit)

if respondToTaskFeeLimit.Cmp(simulatedCost) < 0 {
return fmt.Errorf("cost of transaction is higher than Batch.RespondToTaskFeeLimit")
// NOTE we are not using tx.Cost() because tx.Cost() includes tx.Value()
txCost := new(big.Int).Mul(big.NewInt(int64(tx.Gas())), tx.GasPrice())

if respondToTaskFeeLimit.Cmp(txCost) < 0 {
aggregatorDifferencePaid := new(big.Int).Sub(txCost, respondToTaskFeeLimit)
aggregatorDifferencePaidInEth := utils.WeiToEth(aggregatorDifferencePaid)
w.metrics.AddAggregatorGasPaidForBatcher(aggregatorDifferencePaidInEth)
w.metrics.IncAggregatorPaidForBatcher()
w.logger.Warnf("cost of transaction was higher than Batch.RespondToTaskFeeLimit, aggregator has paid the for the difference, aprox: %vethers", aggregatorDifferencePaidInEth)
}
}

func (w *AvsWriter) checkAggAndBatcherHaveEnoughBalance(tx *types.Transaction, txOpts bind.TransactOpts, batchIdentifierHash [32]byte, senderAddress [20]byte) error {
w.logger.Info("Checking if aggregator and batcher have enough balance for the transaction")
aggregatorAddress := txOpts.From
txCost := new(big.Int).Mul(new(big.Int).SetUint64(tx.Gas()), txOpts.GasPrice)
w.logger.Info("Transaction cost", "cost", txCost)

batchState, err := w.BatchesStateRetryable(&bind.CallOpts{}, batchIdentifierHash)
if err != nil {
w.logger.Error("Failed to get batch state", "error", err)
w.logger.Info("Proceeding to check balances against transaction cost")
return w.compareBalances(txCost, aggregatorAddress, senderAddress)
}
respondToTaskFeeLimit := batchState.RespondToTaskFeeLimit
w.logger.Info("Checking balance against Batch RespondToTaskFeeLimit", "RespondToTaskFeeLimit", respondToTaskFeeLimit)
// Note: we compare both Aggregator funds and Batcher balance in Aligned against respondToTaskFeeLimit
// Batcher will pay up to respondToTaskFeeLimit, for this he needs that amount of funds in Aligned
// Aggregator will pay any extra cost, for this he needs at least respondToTaskFeeLimit in his balance
return w.compareBalances(respondToTaskFeeLimit, aggregatorAddress, senderAddress)
}

Expand Down
10 changes: 5 additions & 5 deletions core/retry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ func TestRespondToTaskV2(t *testing.T) {
}

aggregatorConfig := config.NewAggregatorConfig("../config-files/config-aggregator-test.yaml")
w, err := chainio.NewAvsWriterFromConfig(aggregatorConfig.BaseConfig, aggregatorConfig.EcdsaConfig)
w, err := chainio.NewAvsWriterFromConfig(aggregatorConfig.BaseConfig, aggregatorConfig.EcdsaConfig, nil)
if err != nil {
t.Errorf("Error killing process: %v\n", err)
return
Expand Down Expand Up @@ -734,7 +734,7 @@ func TestBatchesStateWriter(t *testing.T) {
}

aggregatorConfig := config.NewAggregatorConfig("../config-files/config-aggregator-test.yaml")
avsWriter, err := chainio.NewAvsWriterFromConfig(aggregatorConfig.BaseConfig, aggregatorConfig.EcdsaConfig)
avsWriter, err := chainio.NewAvsWriterFromConfig(aggregatorConfig.BaseConfig, aggregatorConfig.EcdsaConfig, nil)
if err != nil {
t.Errorf("Error killing process: %v\n", err)
return
Expand Down Expand Up @@ -784,12 +784,12 @@ func TestBalanceAt(t *testing.T) {
}

aggregatorConfig := config.NewAggregatorConfig("../config-files/config-aggregator-test.yaml")
avsWriter, err := chainio.NewAvsWriterFromConfig(aggregatorConfig.BaseConfig, aggregatorConfig.EcdsaConfig)
avsWriter, err := chainio.NewAvsWriterFromConfig(aggregatorConfig.BaseConfig, aggregatorConfig.EcdsaConfig, nil)
if err != nil {
return
}
aggregator_address := common.HexToAddress("0x0")
blockHeight := big.NewInt(13)
blockHeight := big.NewInt(22)

_, err = avsWriter.BalanceAtRetryable(context.Background(), aggregator_address, blockHeight)
assert.Nil(t, err)
Expand Down Expand Up @@ -831,7 +831,7 @@ func TestBatchersBalances(t *testing.T) {
}

aggregatorConfig := config.NewAggregatorConfig("../config-files/config-aggregator-test.yaml")
avsWriter, err := chainio.NewAvsWriterFromConfig(aggregatorConfig.BaseConfig, aggregatorConfig.EcdsaConfig)
avsWriter, err := chainio.NewAvsWriterFromConfig(aggregatorConfig.BaseConfig, aggregatorConfig.EcdsaConfig, nil)
if err != nil {
return
}
Expand Down
10 changes: 10 additions & 0 deletions core/utils/eth_client_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,16 @@ func BytesToQuorumThresholdPercentages(quorumThresholdPercentagesBytes []byte) e
return quorumThresholdPercentages
}

func WeiToEth(wei *big.Int) float64 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see this is only used for metrics, can't the conversion be handled on the visualization side of those metrics?
This should also not be used for any other purpose other than metrics and logs, because this is a lossy conversion (floats are not good for handling money because of that).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alternatively, why don't we keep the big.Float? That might be lossless (but I'd need to check the docs).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it would be better to handle the conversion from grafana. I'll see if it is possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Oppen I agree no one likes floats, but Prometheus' Gauge accepts only float64 type

weiToEth := new(big.Float).SetFloat64(1e18)
weiFloat := new(big.Float).SetInt(wei)

result := new(big.Float).Quo(weiFloat, weiToEth)
eth, _ := result.Float64()

return eth
}

// Simple algorithm to calculate the gasPrice bump based on:
// the currentGasPrice, a base bump percentage, a retry percentage, and the retry count.
// Formula: currentGasPrice + (currentGasPrice * (baseBumpPercentage + retryCount * incrementalRetryPercentage) / 100)
Expand Down
Loading
Loading