Skip to content
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
90a4e56
#291 extract transaction sender class
otselnik Nov 14, 2021
143913b
#291 move perm accs to transaction sender
otselnik Nov 16, 2021
16c9bff
#291 fix state
otselnik Nov 22, 2021
f2c0303
#291 fix errors
otselnik Nov 23, 2021
57f3b53
Merge remote-tracking branch 'origin/develop' into 291_proxy_refactoring
otselnik Nov 23, 2021
3369f67
#291 merge fixes
otselnik Nov 23, 2021
af721bb
#291 refactoring
otselnik Nov 23, 2021
e512bb8
#291 move EXTRA_GAS to environment
otselnik Nov 23, 2021
8dc5e29
#291 capitalize CONFIRMATION_CHECK_DELAY
otselnik Nov 23, 2021
8dce2a5
#291 sort imports
otselnik Nov 23, 2021
cef8f21
#291 relative paths
otselnik Nov 24, 2021
1bf8383
#291 Should be fixed in #326
otselnik Nov 24, 2021
9672438
#291 testing chnages
otselnik Nov 24, 2021
2d42b73
fix storage account check
sinev-valentine Nov 24, 2021
ac2755c
Merge remote-tracking branch 'origin/develop' into 291_proxy_refactoring
otselnik Nov 24, 2021
3519c61
Merge branch '371_add_FinalizedStorage_to_check' into 291_proxy_refac…
otselnik Nov 24, 2021
bf313a2
#291 rename `trx_with_create_and_airdrop` -> `make_trx_with_create_an…
otselnik Nov 24, 2021
3093fcc
Merge remote-tracking branch 'origin/develop' into 291_proxy_refactoring
otselnik Nov 24, 2021
e3c4e33
#295 fix state
otselnik Nov 24, 2021
9b57e53
#291 iterative combined
otselnik Nov 25, 2021
ccffcf4
#295 do not get measurments
otselnik Nov 25, 2021
4d685db
#291 pull request fixes
otselnik Nov 25, 2021
6f63338
Merge remote-tracking branch 'origin/develop' into 291_proxy_refactoring
otselnik Nov 25, 2021
4546926
Merge branch '291_proxy_refactoring' into 295_iterative_execution
otselnik Nov 25, 2021
7befc6b
#295 turn combined instructions ON
otselnik Nov 29, 2021
b5010f8
#295 make neon_instructions return transasactions
otselnik Nov 29, 2021
5ebf76a
Merge remote-tracking branch 'origin/develop' into 291_proxy_refactoring
otselnik Dec 2, 2021
b464b1e
#291 merge fix
otselnik Dec 2, 2021
79088c6
#295 get rid of `USE_COMBINED_START_CONTINUE`
otselnik Dec 2, 2021
52173a0
Merge branch '291_proxy_refactoring' into 295_iterative_execution
otselnik Dec 2, 2021
4fe15e9
#295 requested fixes
otselnik Dec 2, 2021
607e74e
Merge remote-tracking branch 'origin/develop' into 295_iterative_exec…
otselnik Dec 2, 2021
d9f762a
#295 call_continue_bucked refactoring
otselnik Dec 2, 2021
842ec7d
#295 fix
otselnik Dec 3, 2021
78de121
#295 leave only combined iterative transactions
otselnik Dec 3, 2021
47cf219
#295 move constants into class
otselnik Dec 3, 2021
0eebc1f
#295 refactoring
otselnik Dec 3, 2021
cec7a38
Merge remote-tracking branch 'origin/develop' into 295_iterative_exec…
otselnik Dec 6, 2021
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
61 changes: 57 additions & 4 deletions proxy/common_neon/neon_instruction.py
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ def make_partial_call_instruction(self) -> TransactionInstruction:

AccountMeta(pubkey=SYSVAR_INSTRUCTION_PUBKEY, is_signer=False, is_writable=False),
] + obligatory_accounts
)
)


def make_iterative_call_transaction(self, length_before: int = 0) -> Transaction:
Expand All @@ -311,7 +311,7 @@ def make_iterative_call_transaction(self, length_before: int = 0) -> Transaction
return trx


def make_call_from_account_instruction(self) -> Transaction:
def make_call_from_account_transaction(self) -> Transaction:
return Transaction().add(TransactionInstruction(
program_id = EVM_LOADER_ID,
data = bytearray.fromhex("16") + self.collateral_pool_index_buf + int(0).to_bytes(8, byteorder="little"),
Expand All @@ -332,7 +332,7 @@ def make_call_from_account_instruction(self) -> Transaction:
))


def make_continue_instruction(self, steps, index=None) -> Transaction:
def make_continue_transaction(self, steps, index=None) -> Transaction:
Copy link
Contributor

Choose a reason for hiding this comment

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

The same. Annotations on arguments

data = bytearray.fromhex("14") + self.collateral_pool_index_buf + steps.to_bytes(8, byteorder="little")
if index:
data = data + index.to_bytes(8, byteorder="little")
Expand All @@ -356,7 +356,7 @@ def make_continue_instruction(self, steps, index=None) -> Transaction:
))


def make_cancel_instruction(self) -> Transaction:
def make_cancel_transaction(self) -> Transaction:
return Transaction().add(TransactionInstruction(
program_id = EVM_LOADER_ID,
data = bytearray.fromhex("15") + self.eth_trx.nonce.to_bytes(8, 'little'),
Expand All @@ -373,3 +373,56 @@ def make_cancel_instruction(self) -> Transaction:
AccountMeta(pubkey=SYSVAR_INSTRUCTION_PUBKEY, is_signer=False, is_writable=False),
] + obligatory_accounts
))


def make_partial_call_or_continue_instruction(self, steps: int = 0) -> TransactionInstruction:
Copy link
Contributor

Choose a reason for hiding this comment

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

Annotations

data = bytearray.fromhex("0D") + self.collateral_pool_index_buf + steps.to_bytes(8, byteorder="little") + self.msg
return TransactionInstruction(
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it better if it would be separated on two parts. The part that is passed in - is too long

instraction = 
...
return instruction

Copy link
Contributor

Choose a reason for hiding this comment

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

This remark could be connected to the previous definition as well

program_id = EVM_LOADER_ID,
data = data,
keys = [
AccountMeta(pubkey=self.storage, is_signer=False, is_writable=True),

AccountMeta(pubkey=SYSVAR_INSTRUCTION_PUBKEY, is_signer=False, is_writable=False),
AccountMeta(pubkey=self.operator_account, is_signer=True, is_writable=True),
AccountMeta(pubkey=self.collateral_pool_address, is_signer=False, is_writable=True),
AccountMeta(pubkey=self.operator_neon_address, is_signer=False, is_writable=True),
AccountMeta(pubkey=self.caller_token, is_signer=False, is_writable=True),
AccountMeta(pubkey=SYS_PROGRAM_ID, is_signer=False, is_writable=False),

] + self.eth_accounts + [

AccountMeta(pubkey=SYSVAR_INSTRUCTION_PUBKEY, is_signer=False, is_writable=False),
] + obligatory_accounts
)


def make_partial_call_or_continue_transaction(self, steps: int = 0, length_before: int = 0) -> Transaction:
trx = Transaction()
trx.add(self.make_keccak_instruction(length_before + 1, len(self.eth_trx.unsigned_msg()), 13))
trx.add(self.make_partial_call_or_continue_instruction(steps))
return trx


def make_partial_call_or_continue_from_account_data(self, steps, index=None) -> Transaction:
data = bytearray.fromhex("0E") + self.collateral_pool_index_buf + steps.to_bytes(8, byteorder='little')
if index:
data = data + index.to_bytes(8, byteorder="little")
return Transaction().add(TransactionInstruction(
Copy link
Contributor

Choose a reason for hiding this comment

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

     transaction = Transaction()
     transaction.add(...)
     # or extend_trx_with
     # or instruction = make_continue_instruction
     return transaction()

program_id = EVM_LOADER_ID,
data = data,
keys = [
AccountMeta(pubkey=self.holder, is_signer=False, is_writable=True),
AccountMeta(pubkey=self.storage, is_signer=False, is_writable=True),

AccountMeta(pubkey=self.operator_account, is_signer=True, is_writable=True),
AccountMeta(pubkey=self.collateral_pool_address, is_signer=False, is_writable=True),
AccountMeta(pubkey=self.operator_neon_address, is_signer=False, is_writable=True),
AccountMeta(pubkey=self.caller_token, is_signer=False, is_writable=True),
AccountMeta(pubkey=SYS_PROGRAM_ID, is_signer=False, is_writable=False),

] + self.eth_accounts + [

AccountMeta(pubkey=SYSVAR_INSTRUCTION_PUBKEY, is_signer=False, is_writable=False),
] + obligatory_accounts
))
3 changes: 2 additions & 1 deletion proxy/common_neon/solana_interactor.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import re
import time

from solana.rpc.api import Client as SolanaClient
from solana.rpc.commitment import Confirmed
from solana.rpc.types import TxOpts

Expand All @@ -17,7 +18,7 @@


class SolanaInteractor:
def __init__(self, signer, client) -> None:
def __init__(self, signer, client: SolanaClient) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

signer needs its own annotation as well

self.signer = signer
self.client = client

Expand Down
152 changes: 129 additions & 23 deletions proxy/common_neon/transaction_sender.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import json
import logging
import math
import os
import rlp
import time
Expand Down Expand Up @@ -27,6 +28,11 @@
logger.setLevel(logging.DEBUG)


CONTINUE_REGULAR = 'ContinueV02'
Copy link
Contributor

Choose a reason for hiding this comment

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

class A:
    CONSTANT1 = "Hi Dmitrii!!!"
    
    def foo(self):
        print(f"{self.CONSTANT1}")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need explanation

CONTINUE_COMBINED = 'PartialCallOrContinueFromRawEthereumTX'
CONTINUE_HOLDER_COMB = 'ExecuteTrxFromAccountDataIterativeOrContinue'


class TransactionSender:
def __init__(self, solana_interactor: SolanaInteractor, eth_trx: EthTrx, steps: int) -> None:
self.sender = solana_interactor
Expand Down Expand Up @@ -67,7 +73,7 @@ def execute(self):
try:
if call_iterative:
try:
return iterative_executor.call_signed_iterative()
return iterative_executor.call_signed_iterative_combined()
except Exception as err:
logger.debug(str(err))
if str(err).startswith("transaction too large:"):
Expand All @@ -77,7 +83,7 @@ def execute(self):
raise

if call_from_holder:
return iterative_executor.call_signed_with_holder_acc()
return iterative_executor.call_signed_with_holder_combined()
finally:
self.free_perm_accs()

Expand All @@ -93,7 +99,7 @@ def create_noniterative_executor(self):

def create_iterative_executor(self):
self.instruction.init_iterative(self.storage, self.holder, self.perm_accs_id)
return IterativeTransactionSender(self.sender, self.instruction, self.create_acc_trx, self.eth_trx, self.steps)
return IterativeTransactionSender(self.sender, self.instruction, self.create_acc_trx, self.eth_trx, self.steps, self.steps_emulated)


def init_perm_accs(self):
Expand Down Expand Up @@ -302,6 +308,8 @@ def create_account_list_by_emulate(self):
AccountMeta(pubkey=self.caller_token, is_signer=False, is_writable=True),
] + add_keys_05

self.steps_emulated = output_json["steps_executed"]


class NoniterativeTransactionSender:
def __init__(self, solana_interactor: SolanaInteractor, neon_instruction: NeonInstruction, create_acc_trx: Transaction, eth_trx: EthTrx):
Expand All @@ -321,53 +329,70 @@ def call_signed_noniterative(self):


class IterativeTransactionSender:
def __init__(self, solana_interactor: SolanaInteractor, neon_instruction: NeonInstruction, create_acc_trx: Transaction, eth_trx: EthTrx, steps: int):
def __init__(self, solana_interactor: SolanaInteractor, neon_instruction: NeonInstruction, create_acc_trx: Transaction, eth_trx: EthTrx, steps: int, steps_emulated: int):
self.sender = solana_interactor
self.instruction = neon_instruction
self.create_acc_trx = create_acc_trx
self.eth_trx = eth_trx
self.steps = steps
self.steps_emulated = steps_emulated


def call_signed_iterative(self):
if len(self.create_acc_trx.instructions):
precall_txs = Transaction()
precall_txs.add(self.create_acc_trx)
self.sender.send_measured_transaction(precall_txs, self.eth_trx, 'CreateAccountsForTrx')

call_txs = self.instruction.make_iterative_call_transaction()
''' Deprecated '''
self.create_accounts_for_trx()

logger.debug("Partial call")
call_txs = self.instruction.make_iterative_call_transaction()
self.sender.send_measured_transaction(call_txs, self.eth_trx, 'PartialCallFromRawEthereumTXv02')

return self.call_continue()
return self.call_continue(CONTINUE_REGULAR)


def call_signed_iterative_combined(self):
self.create_accounts_for_trx()

return self.call_continue(CONTINUE_COMBINED)


def call_signed_with_holder_acc(self):
''' Deprecated '''
self.write_trx_to_holder_account()
if len(self.create_acc_trx.instructions):
precall_txs = Transaction()
precall_txs.add(self.create_acc_trx)
self.sender.send_measured_transaction(precall_txs, self.eth_trx, 'create_accounts_for_deploy')
self.create_accounts_for_trx()

# ExecuteTrxFromAccountDataIterative
logger.debug("ExecuteTrxFromAccountDataIterative:")
call_txs = self.instruction.make_call_from_account_instruction()
call_txs = self.instruction.make_call_from_account_transaction()
self.sender.send_measured_transaction(call_txs, self.eth_trx, 'ExecuteTrxFromAccountDataIterativeV02')

return self.call_continue()
return self.call_continue(CONTINUE_REGULAR)

def call_signed_with_holder_combined(self):
self.write_trx_to_holder_account()
self.create_accounts_for_trx()

return self.call_continue(CONTINUE_HOLDER_COMB)


def create_accounts_for_trx(self):
length = len(self.create_acc_trx.instructions)
if length == 0:
return

logger.debug(f"Create account for trx: {length}")
precall_txs = Transaction()
precall_txs.add(self.create_acc_trx)
self.sender.send_measured_transaction(precall_txs, self.eth_trx, 'CreateAccountsForTrx')


def write_trx_to_holder_account(self):
logger.debug('write_trx_to_holder_account')
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it really need here with no a context or it's rather a spam

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep

msg = self.eth_trx.signature() + len(self.eth_trx.unsigned_msg()).to_bytes(8, byteorder="little") + self.eth_trx.unsigned_msg()

# Write transaction to transaction holder account
offset = 0
receipts = []
rest = msg
while len(rest):
(part, rest) = (rest[:1000], rest[1000:])
# logger.debug("sender_sol %s %s %s", sender_sol, holder, acc.public_key())
trx = self.instruction.make_write_transaction(offset, part)
receipts.append(self.sender.send_transaction_unconfirmed(trx))
offset += len(part)
Expand All @@ -376,7 +401,20 @@ def write_trx_to_holder_account(self):
self.sender.collect_results(receipts, eth_trx=self.eth_trx, reason='WriteHolder')


Copy link
Contributor

Choose a reason for hiding this comment

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

single blank line in python classes separates methods

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we need just some issue for styling. Not here

def call_continue(self):
def call_continue(self, instruction_type):
return_result = None
try:
return_result = self.call_continue_bucked(instruction_type)
except Exception as err:
logger.debug("call_continue_bucked_combined exception: {}".format(str(err)))

if return_result is not None:
return return_result

return self.call_continue_iterative()


def call_continue_iterative(self):
try:
return self.call_continue_step_by_step()
except Exception as err:
Expand All @@ -398,7 +436,7 @@ def call_continue_step_by_step(self):
def call_continue_step(self):
step_count = self.steps
while step_count > 0:
trx = self.instruction.make_continue_instruction(step_count)
trx = self.instruction.make_continue_transaction(step_count)

logger.debug("Step count {}".format(step_count))
try:
Expand All @@ -413,8 +451,76 @@ def call_continue_step(self):


def call_cancel(self):
trx = self.instruction.make_cancel_instruction()
trx = self.instruction.make_cancel_transaction()
Copy link
Contributor

Choose a reason for hiding this comment

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

Stay look wierd.
trx = Transaction();
cancel_instruction = self.instraction.make_cancel_instruction()
trx.add(cancel_instruction)

Here it's not a big work to follow DIP, because instruction shouldn't know any about a transaction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you want you can use this function the same way and it will work.

trx = Transaction();
cancel_instruction = self.instraction.make_cancel_transaction()
trx.add(cancel_instruction)

For me shorter - better

Copy link
Contributor

@rozhkovdmitrii rozhkovdmitrii Dec 3, 2021

Choose a reason for hiding this comment

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

"shorter" tends to get write-only code for us


logger.debug("Cancel")
result = self.sender.send_measured_transaction(trx, self.eth_trx, 'CancelWithNonce')
return result['result']['transaction']['signatures'][0]


def call_continue_bucked(self, instruction_type):
Copy link
Contributor

Choose a reason for hiding this comment

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

Annotations?

logger.debug("Send bucked combined: %s", instruction_type)
Copy link
Contributor

Choose a reason for hiding this comment

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

f python string?

steps = self.steps

receipts = []
for index in range(math.ceil(self.steps_emulated/self.steps) + self.addition_count(instruction_type)):
try:
Copy link
Contributor

Choose a reason for hiding this comment

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

This method is too heave, you can isolate some logically independent parts

trx = self.make_bucked_trx()

receipts.append(self.sender.send_transaction_unconfirmed(trx))
except SendTransactionError as err:
logger.error(f"Failed to call continue bucked, error: {err}")
logger.debug(str(err))
Copy link
Contributor

Choose a reason for hiding this comment

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

logger.error(f"Failed to call continue bucked, error: {err}")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

if str(err).startswith("Transaction simulation failed: Error processing Instruction 0: custom program error: 0x1"):
Copy link
Contributor

Choose a reason for hiding this comment

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

You should extend SolanaErrors enum with it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need explanaition

pass
elif str(err).startswith("Transaction simulation failed: Error processing Instruction 0: custom program error: 0x4"):
Copy link
Contributor

Choose a reason for hiding this comment

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

You should extend SolanaErrors enum with it

On the top of that there in errors.py you left:

  1. Single line separator before SolanaErrors
  2. CamelCase named method getError

pass
elif check_if_program_exceeded_instructions(err.result):
steps = int(steps * 90 / 100)
else:
raise
except Exception as err:
logger.debug(str(err))
Copy link
Contributor

Choose a reason for hiding this comment

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

f"Failed to call continue bucked, error: {err}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

if str(err).startswith('failed to get recent blockhash'):
Copy link
Contributor

Choose a reason for hiding this comment

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

Another one "umbrella" - SolanaError?

pass
else:
raise

return self.collect_bucked_results(receipts, instruction_type)


@staticmethod
def addition_count(instruction_type):
'''
How many transactions are needed depending on trx type:
CONTINUE_COMBINED: 2 (1 for begin and 1 for decreased steps)
CONTINUE_HOLDER_COMB: 1 for begin
0 otherwise
'''
addition_count = 0
if instruction_type == CONTINUE_COMBINED:
addition_count = 2
elif instruction_type == CONTINUE_HOLDER_COMB:
addition_count = 1
return addition_count


def make_bucked_trx(self, instruction_type, steps, index):
if instruction_type == CONTINUE_REGULAR:
return self.instruction.make_continue_transaction(steps, index)
elif instruction_type == CONTINUE_COMBINED:
return self.instruction.make_partial_call_or_continue_transaction(steps - index)
elif instruction_type == CONTINUE_HOLDER_COMB:
return self.instruction.make_partial_call_or_continue_from_account_data(steps, index)
else:
raise Exception("Unknown continue type: {}".format(instruction_type))


def collect_bucked_results(self, receipts, reason):
logger.debug(f"Collected bucked results: {receipts}")
result_list = self.sender.collect_results(receipts, eth_trx=self.eth_trx, reason=reason)
for result in result_list:
# self.sender.get_measurements(result)
Copy link
Contributor

@rozhkovdmitrii rozhkovdmitrii Dec 2, 2021

Choose a reason for hiding this comment

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

It is not needed more

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Disagree

signature = check_if_continue_returned(result)
if signature:
return signature
1 change: 0 additions & 1 deletion proxy/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

NEW_USER_AIRDROP_AMOUNT = int(os.environ.get("NEW_USER_AIRDROP_AMOUNT", "0"))
CONFIRMATION_CHECK_DELAY = float(os.environ.get("NEON_CONFIRMATION_CHECK_DELAY", "0.1"))
USE_COMBINED_START_CONTINUE = os.environ.get("USE_COMBINED_START_CONTINUE", "NO") == "YES"
CONTINUE_COUNT_FACTOR = int(os.environ.get("CONTINUE_COUNT_FACTOR", "3"))
TIMEOUT_TO_RELOAD_NEON_CONFIG = int(os.environ.get("TIMEOUT_TO_RELOAD_NEON_CONFIG", "3600"))
MINIMAL_GAS_PRICE=int(os.environ.get("MINIMAL_GAS_PRICE", 1))*10**9
Expand Down
1 change: 0 additions & 1 deletion proxy/run-proxy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ if [ "$CONFIG" == "ci" ]; then
[[ -z "$SOLANA_URL" ]] && export SOLANA_URL="http://solana:8899"
[[ -z "$EXTRA_GAS" ]] && export EXTRA_GAS=100000
[[ -z "$NEON_CLI_TIMEOUT" ]] && export NEON_CLI_TIMEOUT="0.5"
[[ -z "$USE_COMBINED_START_CONTINUE" ]] && export USE_COMBINED_START_CONTINUE="NO"
[[ -z "$CONTINUE_COUNT_FACTOR" ]] && export CONTINUE_COUNT_FACTOR="3"
[[ -z "$MINIMAL_GAS_PRICE" ]] && export MINIMAL_GAS_PRICE=0
[[ -z "$POSTGRES_HOST" ]] && export POSTGRES_HOST="postgres"
Expand Down