multiply non-single execution spends by the number of participants#19666
multiply non-single execution spends by the number of participants#19666EasterTheBunny merged 4 commits intodevelopfrom
Conversation
EasterTheBunny
force-pushed
the
CRE-1007/capability-don-metering-support
branch
2 times, most recently
from
961a01e to
737cc4a
Compare
|
I see you updated files related to
|
product-security-plaid-production
Bot
requested review from
MStreet3,
chudilka1,
ecPablo and
ilija42
| github.com/smartcontractkit/chainlink-automation v0.8.1 | ||
| github.com/smartcontractkit/chainlink-ccip v0.1.1-solana.0.20250930202440-88c08e65d960 | ||
| github.com/smartcontractkit/chainlink-common v0.9.6-0.20251002161429-f0c57dab20d5 | ||
| github.com/smartcontractkit/chainlink-common v0.9.6-0.20251006170433-415d51d8f72b |
There was a problem hiding this comment.
there have been other bumps of common, so lets just make sure we pull from develop before merging so we dont regress by accident
There was a problem hiding this comment.
This was the tip of develop at the time of the last commit. The merge will be blocked anyways by a merge conflict on common.
| metadata.CapDON_N = 1 | ||
| } | ||
|
|
||
| if !isGasSpendType(unit) { |
There was a problem hiding this comment.
/nit add ticket num where we handle this more flexibly by injecting whether or not its a single execution cap into the wf registry config?
| } | ||
|
|
||
| if !isGasSpendType(unit) { | ||
| value = value.Mul(decimal.NewFromUint64(uint64(metadata.CapDON_N))) |
There was a problem hiding this comment.
we cast to uint32 prior, now uint64 here - should the type in the proto be set to the equivalent of a unit64 to streamline all these conversions?
There was a problem hiding this comment.
Many of the libraries in Go tend to use the 64 bit variants. It might make sense to streamline as 64 bit if memory size isn't an issue.
| } | ||
|
|
||
| stepDetails.Nodes = nodeDetails | ||
| stepDetails.CapdonN = step.CapDONN |
There was a problem hiding this comment.
could we standardize the variable casing? its a lil tricky to read
There was a problem hiding this comment.
The grpc generated code does camel case but I don't know a way to get upper case inside the name like CapDON. The idiomatic Go casing is to capitalize words like DON and URL. We could certainly pick the grpc variant since we don't really have control of it and end up with strange casing throughout due to the effects of code generation.
| AggSpendValue: "42.0000000000", | ||
| AggSpendUnit: billing.ResourceType_RESOURCE_TYPE_COMPUTE.String(), | ||
| AggSpendValueCre: "84.0000000000", | ||
| AggSpendValueCre: "840.0000000000", |
There was a problem hiding this comment.
do we have corresponding coverage for gas?
i.e. a Settle call on where the number of credits is not multiplied by CapDON_N because its gas?
There was a problem hiding this comment.
It does, but I'll make it a more explicit test to communicate the purpose better.
EasterTheBunny
force-pushed
the
CRE-1007/capability-don-metering-support
branch
from
c99d816 to
78e514f
Compare
| {Peer2PeerID: "lmno", SpendUnit: billing.ResourceType_RESOURCE_TYPE_COMPUTE.String(), SpendValue: "12"}, | ||
| }})) | ||
|
|
||
| expected[stepRef] = &eventspb.MeteringReportStep{ |
There was a problem hiding this comment.
how is expected used?
There was a problem hiding this comment.
It is used in a direct equality check with the output of FormatReport
assert.Equal(t, expected, report.FormatReport().Steps)
|
3 participants
CRE-1007
Requires
Supports