Skip to content

Commit fd8dd28

Browse files
tinker-michaeljtinker-michaelj
authored
fix: finalize stake metadata only once per transaction (#12537)
Signed-off-by: Michael Tinker <[email protected]>
1 parent 26de77b commit fd8dd28

File tree

13 files changed

+103
-75
lines changed

13 files changed

+103
-75
lines changed

‎hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/HandleContextImpl.java

+9-35
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
import static com.hedera.node.app.state.HederaRecordCache.DuplicateCheckResult.NO_DUPLICATE;
2929
import static com.hedera.node.app.workflows.handle.HandleContextImpl.PrecedingTransactionCategory.LIMITED_CHILD_RECORDS;
3030
import static com.hedera.node.app.workflows.handle.HandleWorkflow.extraRewardReceivers;
31-
import static java.util.Collections.emptySet;
31+
import static java.util.Collections.emptyMap;
3232
import static java.util.Objects.requireNonNull;
3333

3434
import com.hedera.hapi.node.base.AccountID;
@@ -104,10 +104,10 @@
104104
import edu.umd.cs.findbugs.annotations.NonNull;
105105
import edu.umd.cs.findbugs.annotations.Nullable;
106106
import java.time.Instant;
107-
import java.util.LinkedHashSet;
107+
import java.util.LinkedHashMap;
108108
import java.util.List;
109+
import java.util.Map;
109110
import java.util.Objects;
110-
import java.util.Set;
111111
import java.util.function.Function;
112112
import java.util.function.Predicate;
113113
import java.util.function.Supplier;
@@ -156,7 +156,7 @@ public class HandleContextImpl implements HandleContext, FeeContext {
156156
private AttributeValidator attributeValidator;
157157
private ExpiryValidator expiryValidator;
158158
private ExchangeRateInfo exchangeRateInfo;
159-
private Set<AccountID> dispatchPaidStakerIds;
159+
private Map<AccountID, Long> dispatchPaidRewards;
160160
private PlatformState platformState;
161161

162162
/**
@@ -644,8 +644,8 @@ private <T> T doDispatchChildTransaction(
644644
return castRecordBuilder(childRecordBuilder, recordBuilderClass);
645645
}
646646

647-
public @NonNull Set<AccountID> dispatchPaidStakerIds() {
648-
return dispatchPaidStakerIds == null ? emptySet() : dispatchPaidStakerIds;
647+
public @NonNull Map<AccountID, Long> dispatchPaidRewards() {
648+
return dispatchPaidRewards == null ? emptyMap() : dispatchPaidRewards;
649649
}
650650

651651
private void dispatchSyntheticTxn(
@@ -783,36 +783,10 @@ private void dispatchSyntheticTxn(
783783
payer, finalizeContext, function, extraRewardReceivers(txBody, function, childRecordBuilder));
784784
final var paidStakingRewards = childRecordBuilder.getPaidStakingRewards();
785785
if (!paidStakingRewards.isEmpty()) {
786-
if (dispatchPaidStakerIds == null) {
787-
dispatchPaidStakerIds = new LinkedHashSet<>();
786+
if (dispatchPaidRewards == null) {
787+
dispatchPaidRewards = new LinkedHashMap<>();
788788
}
789-
paidStakingRewards.forEach(aa -> dispatchPaidStakerIds.add(aa.accountIDOrThrow()));
790-
}
791-
} else {
792-
final var finalizeContext = new ChildFinalizeContextImpl(
793-
new ReadableStoreFactory(childStack),
794-
new WritableStoreFactory(childStack, TokenService.NAME),
795-
childRecordBuilder);
796-
childRecordFinalizer.finalizeChildRecord(finalizeContext, function);
797-
}
798-
// For mono-service fidelity, we need to attach staking rewards for a
799-
// triggered transaction to the record of the child here, and not the
800-
// "parent" ScheduleCreate or ScheduleSign transaction
801-
if (childCategory == SCHEDULED) {
802-
final var finalizeContext = new TriggeredFinalizeContext(
803-
new ReadableStoreFactory(childStack),
804-
new WritableStoreFactory(childStack, TokenService.NAME),
805-
childRecordBuilder,
806-
consensusNow(),
807-
configuration);
808-
parentRecordFinalizer.finalizeParentRecord(
809-
payer, finalizeContext, function, extraRewardReceivers(txBody, function, childRecordBuilder));
810-
final var paidStakingRewards = childRecordBuilder.getPaidStakingRewards();
811-
if (!paidStakingRewards.isEmpty()) {
812-
if (dispatchPaidStakerIds == null) {
813-
dispatchPaidStakerIds = new LinkedHashSet<>();
814-
}
815-
paidStakingRewards.forEach(aa -> dispatchPaidStakerIds.add(aa.accountIDOrThrow()));
789+
paidStakingRewards.forEach(aa -> dispatchPaidRewards.put(aa.accountIDOrThrow(), aa.amount()));
816790
}
817791
} else {
818792
final var finalizeContext = new ChildFinalizeContextImpl(

‎hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/HandleWorkflow.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import static com.hedera.node.app.workflows.prehandle.PreHandleResult.Status.PRE_HANDLE_FAILURE;
4747
import static com.hedera.node.app.workflows.prehandle.PreHandleResult.Status.SO_FAR_SO_GOOD;
4848
import static java.util.Collections.emptyList;
49+
import static java.util.Collections.emptyMap;
4950
import static java.util.Collections.emptySet;
5051
import static java.util.Objects.requireNonNull;
5152

@@ -126,6 +127,7 @@
126127
import java.util.EnumSet;
127128
import java.util.LinkedHashSet;
128129
import java.util.List;
130+
import java.util.Map;
129131
import java.util.Set;
130132
import java.util.concurrent.atomic.AtomicBoolean;
131133
import javax.inject.Inject;
@@ -355,7 +357,7 @@ private void handleUserTransaction(
355357
AccountID payer = null;
356358
Fees fees = null;
357359
TransactionInfo transactionInfo = null;
358-
Set<AccountID> prePaidRewardReceivers = emptySet();
360+
Map<AccountID, Long> prePaidRewards = emptyMap();
359361
try {
360362
final var preHandleResult = getCurrentPreHandleResult(readableStoreFactory, creator, platformTxn);
361363

@@ -549,7 +551,7 @@ private void handleUserTransaction(
549551
recordBuilder.status(SUCCESS);
550552
// Only ScheduleCreate and ScheduleSign can trigger paid staking rewards via
551553
// dispatch; and only if this top-level transaction was successful
552-
prePaidRewardReceivers = context.dispatchPaidStakerIds();
554+
prePaidRewards = context.dispatchPaidRewards();
553555

554556
// Notify responsible facility if system-file was uploaded.
555557
// Returns SUCCESS if no system-file was uploaded
@@ -625,7 +627,7 @@ private void handleUserTransaction(
625627
tokenServiceContext,
626628
function,
627629
extraRewardReceivers(transactionInfo, recordBuilder),
628-
prePaidRewardReceivers);
630+
prePaidRewards);
629631

630632
// Commit all state changes
631633
stack.commitFullStack();

‎hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/TokenContextImpl.java

+5
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,11 @@ public boolean isFirstTransaction() {
131131
return isFirstTransaction;
132132
}
133133

134+
@Override
135+
public boolean isScheduleDispatch() {
136+
return false;
137+
}
138+
134139
@Override
135140
public void markMigrationRecordsStreamed() {
136141
blockRecordManager.markMigrationRecordsStreamed();

‎hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/TriggeredFinalizeContext.java

+5
Original file line numberDiff line numberDiff line change
@@ -67,4 +67,9 @@ public boolean hasChildRecords() {
6767
public <T> void forEachChildRecord(@NonNull Class<T> recordBuilderClass, @NonNull Consumer<T> consumer) {
6868
// No-op, as contract operations cannot be scheduled at this time
6969
}
70+
71+
@Override
72+
public boolean isScheduleDispatch() {
73+
return true;
74+
}
7075
}

‎hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/FinalizeParentRecordHandler.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public void finalizeParentRecord(
7777
@NonNull final FinalizeContext context,
7878
@NonNull final HederaFunctionality functionality,
7979
@NonNull final Set<AccountID> explicitRewardReceivers,
80-
@NonNull final Set<AccountID> prePaidRewardReceivers) {
80+
@NonNull final Map<AccountID, Long> prePaidRewards) {
8181
final var recordBuilder = context.userTransactionRecordBuilder(CryptoTransferRecordBuilder.class);
8282

8383
// This handler won't ask the context for its transaction, but instead will determine the net hbar transfers and
@@ -97,7 +97,7 @@ public void finalizeParentRecord(
9797
// Calculate staking rewards and add them also to hbarChanges here, before assessing
9898
// net changes for transaction record
9999
final var rewardsPaid =
100-
stakingRewardsHandler.applyStakingRewards(context, explicitRewardReceivers, prePaidRewardReceivers);
100+
stakingRewardsHandler.applyStakingRewards(context, explicitRewardReceivers, prePaidRewards);
101101
if (requiresExternalization(rewardsPaid)) {
102102
recordBuilder.paidStakingRewards(asAccountAmounts(rewardsPaid));
103103
}

‎hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/staking/StakingRewardsHandler.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,13 @@ public interface StakingRewardsHandler {
4949
*
5050
* @param context the context of the transaction
5151
* @param explicitRewardReceivers a set of accounts that must be considered for rewards independent of the context
52-
* @param prePaidRewardReceivers a set of accounts that have already been paid rewards in the current transaction
52+
* @param prePaidRewards a set of accounts that have already been paid rewards in the current transaction
5353
* @return a map of account id to the amount of rewards paid out
5454
*/
5555
Map<AccountID, Long> applyStakingRewards(
5656
FinalizeContext context,
5757
@NonNull Set<AccountID> explicitRewardReceivers,
58-
@NonNull Set<AccountID> prePaidRewardReceivers);
58+
@NonNull Map<AccountID, Long> prePaidRewards);
5959

6060
/**
6161
* Checks if the account has been rewarded since the last staking metadata change.

‎hedera-node/hedera-token-service-impl/src/main/java/com/hedera/node/app/service/token/impl/handlers/staking/StakingRewardsHandlerImpl.java

+19-8
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public StakingRewardsHandlerImpl(
7171
public Map<AccountID, Long> applyStakingRewards(
7272
@NonNull final FinalizeContext context,
7373
@NonNull final Set<AccountID> explicitRewardReceivers,
74-
@NonNull final Set<AccountID> prePaidRewardReceivers) {
74+
@NonNull final Map<AccountID, Long> prePaidRewards) {
7575
requireNonNull(context);
7676
requireNonNull(explicitRewardReceivers);
7777
final var writableStore = context.writableStore(WritableAccountStore.class);
@@ -87,17 +87,28 @@ public Map<AccountID, Long> applyStakingRewards(
8787
// get list of possible reward receivers which are staked to node
8888
final var rewardReceivers =
8989
getAllRewardReceivers(writableStore, stakedToMeRewardReceivers, explicitRewardReceivers);
90-
rewardReceivers.removeAll(prePaidRewardReceivers);
90+
// We don't want to repeat any rewards that have already been paid (in current implementation, this means during
91+
// a SCHEDULED dispatch)
92+
rewardReceivers.removeAll(prePaidRewards.keySet());
9193
// Pay rewards to all possible reward receivers, returns all rewards paid
9294
final var recordBuilder = context.userTransactionRecordBuilder(DeleteCapableTransactionRecordBuilder.class);
9395
final var rewardsPaid = rewardsPayer.payRewardsIfPending(
9496
rewardReceivers, writableStore, stakingRewardsStore, stakingInfoStore, consensusNow, recordBuilder);
95-
// Apply all changes related to stakedId changes, and adjust stakedToMe
96-
// for all accounts staking to an account
97-
adjustStakedToMeForAccountStakees(writableStore);
98-
// Adjust stakes for nodes and account's staking metadata
99-
adjustStakeMetadata(
100-
writableStore, stakingInfoStore, stakingRewardsStore, consensusNow, rewardsPaid, rewardReceivers);
97+
98+
if (!context.isScheduleDispatch()) {
99+
// We only manage stake metadata once, at the end of a transaction; but to do
100+
// this correctly, we need to include information about any rewards paid during
101+
// a SCHEDULED dispatch
102+
rewardReceivers.addAll(prePaidRewards.keySet());
103+
rewardsPaid.putAll(prePaidRewards);
104+
// Apply all changes related to stakedId changes, and adjust stakedToMe
105+
// for all accounts staking to an account
106+
adjustStakedToMeForAccountStakees(writableStore);
107+
// Adjust stakes for nodes and account's staking metadata
108+
adjustStakeMetadata(
109+
writableStore, stakingInfoStore, stakingRewardsStore, consensusNow, rewardsPaid, rewardReceivers);
110+
}
111+
101112
// Decrease staking reward account balance by rewardPaid amount
102113
decreaseStakeRewardAccountBalance(rewardsPaid, stakingRewardAccountId, writableStore);
103114
return rewardsPaid;

‎hedera-node/hedera-token-service-impl/src/test/java/com/hedera/node/app/service/token/impl/test/handlers/FinalizeParentRecordHandlerTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -889,7 +889,7 @@ void handleNftTransfersToExistingAccountSuccess() {
889889
subject.finalizeParentRecord(
890890
ACCOUNT_1212_ID, context, HederaFunctionality.CRYPTO_DELETE, Collections.emptySet());
891891
verify(stakingRewardsHandler, never())
892-
.applyStakingRewards(context, Collections.emptySet(), Collections.emptySet());
892+
.applyStakingRewards(context, Collections.emptySet(), Collections.emptyMap());
893893
}
894894

895895
@Test

0 commit comments

Comments
 (0)