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(eth): proper gas calculation for eth_getBlockReceipts #12514

Merged
merged 3 commits into from
Sep 26, 2024

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Sep 26, 2024

Ref: #12505

@rvagg rvagg force-pushed the rvagg/ethgetblockreceipts-calc branch from 972158b to 73fba51 Compare September 26, 2024 05:08
@rvagg rvagg added the skip/changelog This change does not require CHANGELOG.md update label Sep 26, 2024
@rvagg
Copy link
Member Author

rvagg commented Sep 26, 2024

Oh, the failure is interesting

    eth_legacy_transactions_test.go:362: 
        	Error Trace:	/home/runner/work/lotus/lotus/itests/eth_legacy_transactions_test.go:362
        	Error:      	Not equal: 
        	            	expected: 229215
        	            	actual  : 231755
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -4,3 +4,3 @@
        	            	   abs: (big.nat) (len=1) {
        	            	-   (big.Word) 229215
        	            	+   (big.Word) 231755
        	            	   }
        	Test:       	TestLegacyContractInvocation

why is that not happy? I shouldn't have changed EthGetTransactionReceipt ..

@rvagg
Copy link
Member Author

rvagg commented Sep 26, 2024

ok, 🤞 let's try that change, I didn't go back to the parents to get the parentbasefee

@@ -527,7 +527,20 @@ func (a *EthModule) EthGetTransactionReceiptLimited(ctx context.Context, txHash
return nil, xerrors.Errorf("failed to convert %s into an Eth Txn: %w", txHash, err)
}

receipt, err := newEthTxReceipt(ctx, tx, msgLookup, a.ChainAPI, a.EthEventHandler)
ts, err := a.Chain.GetTipSetFromKey(ctx, msgLookup.TipSet)
Copy link
Contributor

Choose a reason for hiding this comment

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

Right so msgLookup.Tipset is the "execution tipset" of the message and NOT the "inclusion tipset".

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, and the base fee comes off the inclusion one

or something like that

@rvagg rvagg merged commit f8857f1 into feat/eth-block-get-receipts-api Sep 26, 2024
76 checks passed
@rvagg rvagg deleted the rvagg/ethgetblockreceipts-calc branch September 26, 2024 05:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip/changelog This change does not require CHANGELOG.md update
Projects
Status: 🎉 Done
Development

Successfully merging this pull request may close these issues.

2 participants