SetConfig optimization + billing config improvements #36

archseer · 2022-01-20T08:48:16Z

Need to fit each call into 4kb

[please don't squash]

archseer · 2022-01-20T08:49:09Z

contracts/ocr2/src/contract.rs

+            base_gas: None,
+            gas_per_signature: None,
+            gas_adjustment: None,


These are now settable via setBilling but can also be nil to use the built-in default

archseer · 2022-01-20T08:50:12Z

contracts/ocr2/src/contract.rs

+    let gas_per_signature = decimal(config.gas_per_signature.unwrap_or(17_000));
+    let base_gas = decimal(config.base_gas.unwrap_or(84_000));
+    let gas_adjustment = Decimal::percent(u64::from(config.gas_adjustment.unwrap_or(140)));


Adjusted based on devnet values. We seemed to over-estimate by almost exactly 4x. The amount charged is also the "gas allocated", not "gas used" and the difference seems to be about 1.4x

contracts/ocr2/src/msg.rs

contracts/ocr2/src/contract.rs

pkg/terra/contract_transmitter.go

contracts/ocr2/src/msg.rs

contracts/ocr2/src/integration_tests.rs

contracts/ocr2/src/state.rs

krebernisak · 2022-01-25T12:09:31Z

contracts/ocr2/src/state.rs

 pub struct Billing {
    /// Should match https://fcd.terra.dev/v1/txs/gas_prices
    /// NOTE: needs to be scaled to the same amount of decimals places as LINK token
    pub recommended_gas_price: u64,
    pub observation_payment: u64,
+    pub transmission_payment: u64,
+    pub base_gas: Option<u64>,


I'd suggest we rename this as gas_base to be consistent with other args.

contracts/ocr2/src/state.rs

contracts/ocr2/schema/execute_msg.json

krebernisak · 2022-02-01T11:37:19Z

contracts/ocr2/src/contract.rs

@@ -929,11 +931,15 @@ pub fn execute_set_billing(
        Event::new("set_billing")
            .add_attribute(
                "recommended_gas_price",


Should we rename this attribute as recommended_gas_price_uluna to keep it consistent?

krebernisak · 2022-02-01T11:39:15Z

contracts/ocr2/src/contract.rs

-        // + transmitter gas reimbursement
-        .checked_add(transmitter.payment)?)
+    Ok(
+        Uint128::new(u128::from(config.billing.observation_payment_gjuels) * 10u128.pow(9)) // scale to juels


Consider extracting this scale factor 10u128.pow(9) as a constant.

jmank88 · 2022-02-01T18:45:45Z

@archseer There are two more places signers are handled as <Vec<Vec> still:
https://github.com/smartcontractkit/chainlink-terra/blob/c5e735e9fae73580d330bfde74db64c11bcf9a39/contracts/ocr2/src/contract.rs#L109
https://github.com/smartcontractkit/chainlink-terra/blob/c5e735e9fae73580d330bfde74db64c11bcf9a39/contracts/ocr2/src/contract.rs#L235

archseer · 2022-02-02T02:03:41Z

That's converting/unwrapping from the Binary type, internally we just use byte vecs https://github.com/smartcontractkit/chainlink-terra/blob/c5e735e9fae73580d330bfde74db64c11bcf9a39/contracts/ocr2/src/msg.rs#L30-L32

RodrigoAD · 2022-02-02T12:06:54Z

packages-ts/gauntlet-terra-contracts/src/commands/contracts/ocr2/setBilling.ts

-      observation_payment: new BN(input.observationPaymentGjuels).toNumber(),
+      observation_payment_gjuels: new BN(input.observationPaymentGjuels).toNumber(),
+      transmission_payment_gjuels: new BN(input.transmissionPaymentGjuels).toNumber(),
+      recommended_gas_price_uluna: input.recommendedGasPriceUluna


This should be of Decimal type (https://github.com/smartcontractkit/chainlink-terra/pull/36/files#diff-7593d1da79e988b58adce267ba96df185c0abebbfc17323f0de5e749e6419134R56)

@archseer Which format should we be applying here?

It's a string, you can pass through the value from https://fcd.terra.dev/v1/txs/gas_prices without modifying it

This info comes from the RDD, so we can expect it to come in the right format

This is so we can fit more data into the 4kb payload

Co-authored-by: Kristijan Rebernisak <kristijan.rebernisak@gmail.com>

Fixes #67

Fixes #59

Code existed, but wasn't present in ExecuteMsg

version of deployed contracts

reductionista · 2022-02-02T16:56:43Z

packages-ts/gauntlet-terra-contracts/src/commands/contracts/ocr2/setBilling.ts

@@ -33,7 +33,7 @@ const makeContractInput = async (input: CommandInput): Promise<ContractInput> =>
    config: {
      observation_payment_gjuels: new BN(input.observationPaymentGjuels).toNumber(),
      transmission_payment_gjuels: new BN(input.transmissionPaymentGjuels).toNumber(),
-      recommended_gas_price_uluna: input.recommendedGasPriceUluna
+      recommended_gas_price_uluna: new BN(input.recommendedGasPriceUluna).toString(),


If this works, I think it will have the effect of converting the decimal to an integer, either by truncating or rounding I suppose. Is that really what we want to do?

From: https://github.com/indutny/bn.js/
"Note: decimals are not supported in this library."

Ah, agreed. RDD should store this as a string to avoid losing precision. @RodrigoAD

You're completely right. I'll open a fix

Looks like it's handled in #76

archseer requested a review from aalu1418 January 20, 2022 08:48

archseer commented Jan 20, 2022

View reviewed changes

contracts/ocr2/src/msg.rs Show resolved Hide resolved

RodrigoAD reviewed Jan 20, 2022

View reviewed changes

contracts/ocr2/src/contract.rs Outdated Show resolved Hide resolved

archseer force-pushed the set-config-optimization branch from 32845a2 to b97e842 Compare January 21, 2022 08:47

archseer marked this pull request as ready for review January 21, 2022 12:58

krebernisak reviewed Jan 24, 2022

View reviewed changes

pkg/terra/contract_transmitter.go Outdated Show resolved Hide resolved

contracts/ocr2/src/msg.rs Show resolved Hide resolved

contracts/ocr2/src/integration_tests.rs Show resolved Hide resolved

contracts/ocr2/src/state.rs Outdated Show resolved Hide resolved

krebernisak linked an issue Jan 25, 2022 that may be closed by this pull request

Tx size limit reached on setting config #31

Closed

krebernisak reviewed Jan 25, 2022

View reviewed changes

krebernisak mentioned this pull request Jan 27, 2022

Consider adding additional checks when setting billing config values #58

Open

krebernisak reviewed Jan 27, 2022

View reviewed changes

contracts/ocr2/src/state.rs Outdated Show resolved Hide resolved

krebernisak mentioned this pull request Jan 27, 2022

Scale 'recommended_gas_price' from uLUNA to LUNA #59

Closed

archseer force-pushed the set-config-optimization branch 2 times, most recently from a94ab2e to d32ce28 Compare February 1, 2022 08:05

archseer changed the title ~~SetConfig optimization~~ SetConfig optimization + billing config improvements Feb 1, 2022

krebernisak reviewed Feb 1, 2022

View reviewed changes

reductionista force-pushed the set-config-optimization branch from 57f6c0f to 8ef78a7 Compare February 1, 2022 23:03

RodrigoAD reviewed Feb 2, 2022

View reviewed changes

archseer and others added 9 commits February 3, 2022 00:15

Use Binary (base64) instead of array for some setConfig values

b3eaacc

This is so we can fit more data into the 4kb payload

Use base64 encoding or transmit call too

7f8ced7

Make gas constants configurable, tweak according to devnet values

c441820

Add billing.transmission_payment

249330d

Update pkg/terra/contract_transmitter.go

5c37fe3

Co-authored-by: Kristijan Rebernisak <kristijan.rebernisak@gmail.com>

Reduce the oracle count to 16 in tests to make them pass

d787650

Rebuild schemas with optimized SetConfig changes

f7b266d

Change all Arrays to Buffers

91d2dc6

Use hex encoding for onchain config & offchain config on events

2c6aeea

archseer and others added 14 commits February 3, 2022 00:16

goimports

0ff142c

Add ownership transfer to access_controller

11ca616

Fixes #67

setBilling should accept gjuels for payment values

357f42d

Specify recommended_gas_price as an uLUNA decimal string

6f7f248

Fixes #59

Hook up set_validator_config, regenerate schema

a28a123

Code existed, but wasn't present in ExecuteMsg

Update schemas for access_controller, and gauntlet codeID's for latest

cb8d7ca

version of deployed contracts

Gauntlet format changes for setBilling() call

0eb4de1

Gauntlet changes for set_config

86d36b3

dummy data on setup flow command

1e8e2f1

don't double b64 encode signer

b4d67a9

Rebuild schema

06c79c4

base_gas -> gas_base

0e4c057

recommended_gas_price -> recommended_gas_price_uluna

3e427a8

Add a GIGA constant

02ebdce

archseer force-pushed the set-config-optimization branch from 55f4dbf to 02ebdce Compare February 2, 2022 15:22

archseer requested a review from krebernisak February 2, 2022 15:25

jmank88 approved these changes Feb 2, 2022

View reviewed changes

archseer merged commit 6f4877c into main Feb 2, 2022

archseer deleted the set-config-optimization branch February 2, 2022 15:36

reductionista reviewed Feb 2, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SetConfig optimization + billing config improvements #36

SetConfig optimization + billing config improvements #36

archseer commented Jan 20, 2022 •

edited

archseer Jan 20, 2022

archseer Jan 20, 2022

krebernisak Jan 25, 2022

krebernisak Feb 1, 2022

krebernisak Feb 1, 2022

jmank88 commented Feb 1, 2022

archseer commented Feb 2, 2022

RodrigoAD Feb 2, 2022 •

edited

archseer Feb 2, 2022

RodrigoAD Feb 2, 2022

reductionista Feb 2, 2022 •

edited

archseer Feb 3, 2022

RodrigoAD Feb 3, 2022

archseer Feb 3, 2022

SetConfig optimization + billing config improvements #36

SetConfig optimization + billing config improvements #36

Conversation

archseer commented Jan 20, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmank88 commented Feb 1, 2022

archseer commented Feb 2, 2022

RodrigoAD Feb 2, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reductionista Feb 2, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

archseer commented Jan 20, 2022 •

edited

RodrigoAD Feb 2, 2022 •

edited

reductionista Feb 2, 2022 •

edited