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 engine api: getPayload V2 and V3 now returns correct blockValue #1691

Merged
merged 2 commits into from
Aug 18, 2023
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
5 changes: 5 additions & 0 deletions nimbus/core/sealer.nim
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,11 @@ proc generateExecutionPayload*(engine: SealingEngineRef,
excessBlobGas: excessBlobGas
))

proc blockValue*(engine: SealingEngineRef): UInt256 =
# return sum of reward for feeRecipient for each
# tx included in a block
engine.txPool.blockValue

proc new*(_: type SealingEngineRef,
chain: ChainRef,
ctx: EthContext,
Expand Down
1 change: 1 addition & 0 deletions nimbus/core/tx_pool/tx_desc.nim
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ type

lifeTime*: times.Duration ## Maximum life time of a tx in the system
priceBump*: uint ## Min precentage price when superseding
blockValue*: UInt256 ## Sum of reward received by feeRecipient

param: TxPoolParam ## Getter/Setter parameters

Expand Down
4 changes: 4 additions & 0 deletions nimbus/core/tx_pool/tx_tasks/tx_packer.nim
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ proc runTxCommit(pst: TxPackerStateRef; item: TxItemRef; gasBurned: GasInt)
assert 0 <= gasTip
let reward = gasBurned.u256 * gasTip.uint64.u256
vmState.stateDB.addBalance(xp.chain.feeRecipient, reward)
xp.blockValue += reward

if vmState.generateWitness:
vmState.stateDB.collectWitnessData()
Expand Down Expand Up @@ -153,6 +154,9 @@ proc vmExecInit(xp: TxPoolRef): TxPackerStateRef
# Flush `packed` bucket
xp.bucketFlushPacked

# reset blockValue before adding any tx
xp.blockValue = 0.u256

xp.chain.maxMode = (packItemsMaxGasLimit in xp.pFlags)

if xp.chain.com.daoForkSupport and
Expand Down
23 changes: 5 additions & 18 deletions nimbus/rpc/engine_api.nim
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,6 @@ proc invalidFCU(com: CommonRef, header: EthBlockHeader): ForkchoiceUpdatedRespon
let blockHash = latestValidHash(com.db, parent, com.ttd.get(high(common.BlockNumber)))
invalidFCU(blockHash)

proc txPriorityFee(ttx: TypedTransaction): UInt256 =
try:
let tx = rlp.decode(distinctBase(ttx), Transaction)
return u256(tx.gasPrice * tx.maxPriorityFee)
except RlpError:
doAssert(false, "found TypedTransaction that RLP failed to decode")

# AARDVARK: make sure I have the right units (wei/gwei)
proc sumOfBlockPriorityFees(payload: ExecutionPayloadV1OrV2): UInt256 =
payload.transactions.foldl(a + txPriorityFee(b), UInt256.zero)

template unsafeQuantityToInt64(q: Quantity): int64 =
int64 q

Expand Down Expand Up @@ -201,11 +190,10 @@ proc handle_getPayload(api: EngineApiRef, payloadId: PayloadID): GetPayloadV2Res
meth = "GetPayload", id = payloadId.toHex

var payload: ExecutionPayloadV1OrV2
if not api.get(payloadId, payload):
var blockValue: UInt256
if not api.get(payloadId, blockValue, payload):
raise unknownPayload("Unknown payload")

let blockValue = sumOfBlockPriorityFees(payload)

return GetPayloadV2Response(
executionPayload: payload,
blockValue: blockValue
Expand All @@ -216,20 +204,19 @@ proc handle_getPayloadV3(api: EngineApiRef, com: CommonRef, payloadId: PayloadID
meth = "GetPayload", id = payloadId.toHex

var payload: ExecutionPayloadV3
if not api.get(payloadId, payload):
var blockValue: UInt256
if not api.get(payloadId, blockValue, payload):
raise unknownPayload("Unknown payload")

if not com.isCancunOrLater(fromUnix(payload.timestamp.unsafeQuantityToInt64)):
raise unsupportedFork("payload timestamp is less than Cancun activation")

var
blockValue: UInt256
blobsBundle: BlobsBundleV1

try:
for ttx in payload.transactions:
let tx = rlp.decode(distinctBase(ttx), Transaction)
blockValue += u256(tx.gasPrice * tx.maxPriorityFee)
if tx.networkPayload.isNil.not:
for blob in tx.networkPayload.blobs:
blobsBundle.blobs.add Web3Blob(blob)
Expand Down Expand Up @@ -462,7 +449,7 @@ proc handle_forkchoiceUpdated(sealingEngine: SealingEngineRef,
let payload = res.get

let id = computePayloadId(blockHash, payloadAttrs)
api.put(id, payload)
api.put(id, sealingEngine.blockValue, payload)

info "Created payload for sealing",
id = id.toHex,
Expand Down
41 changes: 28 additions & 13 deletions nimbus/rpc/merge/mergetypes.nim
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type
PayloadItem = object
id: PayloadID
payload: ExecutionPayload
blockValue: UInt256

HeaderItem = object
hash: Hash256
Expand Down Expand Up @@ -83,43 +84,57 @@ proc get*(api: EngineApiRef, hash: Hash256, header: var EthBlockHeader): bool =
return true
false

proc put*(api: EngineApiRef, id: PayloadID, payload: ExecutionPayload) =
api.payloadQueue.put(PayloadItem(id: id, payload: payload))
proc put*(api: EngineApiRef, id: PayloadID,
blockValue: UInt256, payload: ExecutionPayload) =
api.payloadQueue.put(PayloadItem(id: id,
payload: payload, blockValue: blockValue))

proc put*(api: EngineApiRef, id: PayloadID, payload: SomeExecutionPayload) =
api.put(id, payload.executionPayload)
proc put*(api: EngineApiRef, id: PayloadID,
blockValue: UInt256, payload: SomeExecutionPayload) =
api.put(id, blockValue, payload.executionPayload)

proc get*(api: EngineApiRef, id: PayloadID, payload: var ExecutionPayload): bool =
proc get*(api: EngineApiRef, id: PayloadID,
blockValue: var UInt256,
payload: var ExecutionPayload): bool =
for x in api.payloadQueue:
if x.id == id:
payload = x.payload
blockValue = x.blockValue
return true
false

proc get*(api: EngineApiRef, id: PayloadID, payload: var ExecutionPayloadV1): bool =
proc get*(api: EngineApiRef, id: PayloadID,
blockValue: var UInt256,
payload: var ExecutionPayloadV1): bool =
var p: ExecutionPayload
let found = api.get(id, p)
let found = api.get(id, blockValue, p)
doAssert(p.version == Version.V1)
payload = p.V1
return found

proc get*(api: EngineApiRef, id: PayloadID, payload: var ExecutionPayloadV2): bool =
proc get*(api: EngineApiRef, id: PayloadID,
blockValue: var UInt256,
payload: var ExecutionPayloadV2): bool =
var p: ExecutionPayload
let found = api.get(id, p)
let found = api.get(id, blockValue, p)
doAssert(p.version == Version.V2)
payload = p.V2
return found

proc get*(api: EngineApiRef, id: PayloadID, payload: var ExecutionPayloadV3): bool =
proc get*(api: EngineApiRef, id: PayloadID,
blockValue: var UInt256,
payload: var ExecutionPayloadV3): bool =
var p: ExecutionPayload
let found = api.get(id, p)
let found = api.get(id, blockValue, p)
doAssert(p.version == Version.V3)
payload = p.V3
return found

proc get*(api: EngineApiRef, id: PayloadID, payload: var ExecutionPayloadV1OrV2): bool =
proc get*(api: EngineApiRef, id: PayloadID,
blockValue: var UInt256,
payload: var ExecutionPayloadV1OrV2): bool =
var p: ExecutionPayload
let found = api.get(id, p)
let found = api.get(id, blockValue, p)
doAssert(p.version in {Version.V1, Version.V2})
payload = p.V1V2
return found
Expand Down
11 changes: 7 additions & 4 deletions tests/test_merge.nim
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,16 @@ proc testEngineApiSupport() =

suite "Test engine api support":
test "test payload queue":
api.put(id1, ep1)
api.put(id2, ep2)
api.put(id1, 123.u256, ep1)
api.put(id2, 456.u256, ep2)
var eep1, eep2: ExecutionPayloadV1
check api.get(id1, eep1)
check api.get(id2, eep2)
var bv1, bv2: UInt256
check api.get(id1, bv1, eep1)
check api.get(id2, bv2, eep2)
check eep1.gasLimit == ep1.gasLimit
check eep2.gasLimit == ep2.gasLimit
check bv1 == 123.u256
check bv2 == 456.u256

test "test header queue":
api.put(hash1, hdr1)
Expand Down