Skip to content

Fix GetAsOf() for commitment , so that it handles dereferencing #17687

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 31, 2025
Merged

Fix GetAsOf() for commitment , so that it handles dereferencing #17687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5493ae8
remove early returnin GetAsOf() for commitment
antonis19 Oct 27, 2025
924e2f2
add more test cases for historical proofs
antonis19 Oct 27, 2025
f8a7a62
group variable declarations
antonis19 Oct 27, 2025
c22fc36
fix comments
antonis19 Oct 27, 2025
a99042a
special handling of GetAsOf for commitment
antonis19 Oct 30, 2025
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
6 changes: 5 additions & 1 deletion db/state/aggregator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1796,7 +1796,11 @@ func (at *AggregatorRoTx) DebugRangeLatest(tx kv.Tx, domain kv.Domain, from, to
}

func (at *AggregatorRoTx) GetAsOf(name kv.Domain, k []byte, ts uint64, tx kv.Tx) (v []byte, ok bool, err error) {
return at.d[name].GetAsOf(k, ts, tx)
v, ok, err = at.d[name].GetAsOf(k, ts, tx)
if name == kv.CommitmentDomain && !ok {
Copy link
Member

@awskii awskii Oct 31, 2025

Choose a reason for hiding this comment

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

please move !ok as first condition - it just almost slit from me when i started CR

Copy link
Member

@awskii awskii Oct 31, 2025

Choose a reason for hiding this comment

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

but with !ok && commitmentDomain it looks cleaner. Also you have to make sure [Commitment].GetAsOf returns nil, false right before domain reading with proper comment about branch plain keys dereferencing bad reads protection.

Currently this fix wount work - OK=true in case of successful reading from commtiment domain - value will contain references and this fallback will not take place

Copy link
Member Author

@antonis19 antonis19 Oct 31, 2025

Choose a reason for hiding this comment

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

I thought the theory was that that when [Commitment].GetAsOf() returns nil, false then we fall back to func (at *AggregatorRoTx) GetLatest() . I already put back this code in DomainRoTx.GetAsOf() :

	if dt.name == kv.CommitmentDomain {
		// we need to dereference commitment keys to get actual value. DomainRoTx itself does not have
		// pointers to storage and account domains to do the reference. Aggregator tx must be called instead
		return nil, false, nil
	}

Are you saying that if Commitment].GetAsOf() returns ok = true from history, that still needs special handling???

Copy link
Member

@awskii awskii Oct 31, 2025

Choose a reason for hiding this comment

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

yes indeed that was introduced way ago 0774823
no problem then

v, _, ok, err = at.GetLatest(name, k, tx)
}
return v, ok, err
}

func (at *AggregatorRoTx) GetLatest(domain kv.Domain, k []byte, tx kv.Tx) (v []byte, step kv.Step, ok bool, err error) {
Expand Down
1 change: 0 additions & 1 deletion db/state/domain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1457,7 +1457,6 @@ func (dt *DomainRoTx) GetAsOf(key []byte, txNum uint64, roTx kv.Tx) ([]byte, boo
// pointers to storage and account domains to do the reference. Aggregator tx must be called instead
return nil, false, nil
}

var ok bool
v, _, ok, err = dt.GetLatest(key, roTx)
if err != nil {
Expand Down
91 changes: 75 additions & 16 deletions rpc/jsonrpc/eth_call_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,14 @@ import (
"bytes"
"context"
"fmt"
"io"
"math/big"
"math/rand"
"testing"
"time"

"crypto/ecdsa"

"github.com/holiman/uint256"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -135,51 +139,51 @@ func TestGetProof(t *testing.T) {
{
name: "currentBlockNoState",
addr: contractAddr,
blockNum: 4,
blockNum: 6,
},
{
name: "currentBlockEOA",
addr: bankAddr,
blockNum: 4,
blockNum: 6,
},
{
name: "currentBlockNoAccount",
addr: common.HexToAddress("0xdeaddeaddeaddeaddeaddeaddeaddeaddeaddead0"),
blockNum: 4,
blockNum: 6,
},
{
name: "currentBlockWithState",
addr: contractAddr,
blockNum: 4,
blockNum: 6,
storageKeys: []hexutil.Bytes{key(0), key(4), key(8), key(10)},
stateVal: 2,
},
{
name: "currentBlockWithStateAndShortKeys",
addr: contractAddr,
blockNum: 4,
blockNum: 6,
storageKeys: []hexutil.Bytes{{0x0}, {0x4}, {0x8}, {0x0a}},
stateVal: 2,
},
{
name: "currentBlockWithMissingState",
addr: contractAddr,
storageKeys: []hexutil.Bytes{hexutil.FromHex("0xdeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddead")},
blockNum: 4,
blockNum: 6,
stateVal: 0,
},
{
name: "currentBlockEOAMissingState",
addr: bankAddr,
storageKeys: []hexutil.Bytes{hexutil.FromHex("0xdeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddead")},
blockNum: 4,
blockNum: 6,
stateVal: 0,
},
{
name: "currentBlockNoAccountMissingState",
addr: common.HexToAddress("0xdeaddeaddeaddeaddeaddeaddeaddeaddeaddead0"),
storageKeys: []hexutil.Bytes{hexutil.FromHex("0xdeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddeaddead")},
blockNum: 4,
blockNum: 6,
stateVal: 0,
},
{
Expand All @@ -192,14 +196,18 @@ func TestGetProof(t *testing.T) {
{
name: "notCreatedYetAccount",
addr: receiverAddress, // receiver address only starts existing at block 4
blockNum: 2,
blockNum: 3,
},
{
name: "createdAccountAtBlock", // account created at block 4, proof requested at block 4, latest=6
addr: receiverAddress, // receiver address only starts existing at block 4
blockNum: 4,
},
{
name: "createdAccountBlockAfter", // account created at block 4, proof requested at block 5, latest=6
addr: receiverAddress, // receiver address only starts existing at block 4
blockNum: 5,
},
// {
// name: "tooOldBlock",
// addr: contractAddr,
// blockNum: 1,
// expectedErr: "requested block is too old, block must be within 1 blocks of the head block number (currently 3)",
// },
}

for _, tt := range tests {
Expand Down Expand Up @@ -519,8 +527,29 @@ func contractInvocationData(val byte) []byte {
return hexutil.MustDecode(fmt.Sprintf("0x%x00000000000000000000000000000000000000000000000000000000000000%02x", contractFuncSelector, val))
}

func generatePseudoRandomECDSAKey(rand io.Reader) (*ecdsa.PrivateKey, error) {
return ecdsa.GenerateKey(crypto.S256(), rand)
}

func generatePseudoRandomECDSAKeyPairs(rand io.Reader, n int) ([]*ecdsa.PrivateKey, []*ecdsa.PublicKey, error) {
privateKeys := make([]*ecdsa.PrivateKey, n)
publicKeys := make([]*ecdsa.PublicKey, n)
var err error
for i := 0; i < n; i++ {
privateKeys[i], err = generatePseudoRandomECDSAKey(rand)
if err != nil {
return nil, nil, err
}
publicKeys[i] = &privateKeys[i].PublicKey
}
return privateKeys, publicKeys, nil
}

func chainWithDeployedContract(t *testing.T) (*mock.MockSentry, common.Address, common.Address, common.Address) {
var (
seed = int64(12345)
rng = rand.New(rand.NewSource(seed)) // rng for filler accounts
nFillerAccounts = 400 // nr. of accounts to fill up MPT
signer = types.LatestSignerForChainID(nil)
bankKey, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")
bankAddress = crypto.PubkeyToAddress(bankKey.PublicKey)
Expand All @@ -534,12 +563,16 @@ func chainWithDeployedContract(t *testing.T) (*mock.MockSentry, common.Address,
//Alloc: types.GenesisAlloc{bankAddress: {Balance: bankFunds, Storage: map[common.Hash]common.Hash{crypto.Keccak256Hash([]byte{0x1}): crypto.Keccak256Hash([]byte{0xf})}}}, // TODO (antonis19)
}
)
// accounts to fill up MPT
_, fillerPublicKeys, err := generatePseudoRandomECDSAKeyPairs(rng, nFillerAccounts)
require.NoError(t, err)

m := mock.MockWithGenesis(t, gspec, bankKey, false)
db := m.DB

var contractAddr common.Address

chain, err := core.GenerateChain(m.ChainConfig, m.Genesis, m.Engine, m.DB, 4, func(i int, block *core.BlockGen) {
chain, err := core.GenerateChain(m.ChainConfig, m.Genesis, m.Engine, m.DB, 6, func(i int, block *core.BlockGen) {
nonce := block.TxNonce(bankAddress)
switch i {
case 0:
Expand All @@ -551,15 +584,41 @@ func chainWithDeployedContract(t *testing.T) (*mock.MockSentry, common.Address,
txn, err := types.SignTx(types.NewTransaction(nonce, contractAddr, new(uint256.Int), 900000, new(uint256.Int), contractInvocationData(1)), *signer, bankKey)
require.NoError(t, err)
block.AddTx(txn)
// send txs to filler addresses, so that MPT may be populated ( populate only half in this block, to not exceed gas limit)
nonce++
for idx := 0; idx < nFillerAccounts/2; idx++ {
transferAmount := big.NewInt(1e1)
fillerAddress := crypto.PubkeyToAddress(*fillerPublicKeys[idx])
txn, err := types.SignTx(types.NewTransaction(nonce, fillerAddress, uint256.MustFromBig(transferAmount), 21000, new(uint256.Int), nil), *signer, bankKey)
require.NoError(t, err)
block.AddTx(txn)
nonce++
}
case 2:
txn, err := types.SignTx(types.NewTransaction(nonce, contractAddr, new(uint256.Int), 900000, new(uint256.Int), contractInvocationData(2)), *signer, bankKey)
require.NoError(t, err)
block.AddTx(txn)
// send txs to filler addresses, so that MPT may be populated
// ( populate the second half in this block)
nonce++
for idx := nFillerAccounts / 2; idx < nFillerAccounts; idx++ {
transferAmount := big.NewInt(1e1)
fillerAddress := crypto.PubkeyToAddress(*fillerPublicKeys[idx])
txn, err := types.SignTx(types.NewTransaction(nonce, fillerAddress, uint256.MustFromBig(transferAmount), 21000, new(uint256.Int), nil), *signer, bankKey)
require.NoError(t, err)
block.AddTx(txn)
nonce++
}

case 3:
transferAmount := big.NewInt(1e2)
txn, err := types.SignTx(types.NewTransaction(nonce, receiverAddress, uint256.MustFromBig(transferAmount), 21000, new(uint256.Int), nil), *signer, bankKey)
require.NoError(t, err)
block.AddTx(txn)
case 4:
// empty block
case 5:
// empty block
}
})
if err != nil {
Expand Down
Loading